-
Notifications
You must be signed in to change notification settings - Fork 154
Fixed #176 - Show only active CMS Blocks #221
Conversation
$blockData = $this->blockDataProvider->getData($blockIdentifier); | ||
if (!empty($blockData)) { | ||
$blocksData[$blockIdentifier] = $blockData; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to log the fact that some of the requested blocks were not found, please add this to the else
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I provided a little refactoring of this PR
Now blocks data is located in data
section, and errors are located in errors
section
Also, I removed the logging, because there it’s way to log overflow logfile if I send non-existent block id
And at the same time for the client, there is no information what is the actual error (error is only on the server)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ronak2ram thank you for making all necessary changes. The PR looks good now.
Result: Request
Response
|
-- Fix static tests
-- Fix static tests
* @param \Exception|null $cause | ||
* @param int $code | ||
*/ | ||
public function __construct(string $message, array $responseData, \Exception $cause = null, int $code = 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not swap the original arguments order to comply with Liskov substitution principle.
Looks like response structure does not match the request structure after the fix (there is additional |
Description
Removed only active block checking code.
Fixed Issues (if relevant)
Steps to reproduce
Pull request #31 CMS page/block coverage #105
Enable Block: NO -> Save
disabled_cms_block: identifier of the disabled cms_block
enabled_cms_block: identifier of the enabled cms_block
Expected result
Actual result
Contribution checklist