-
Notifications
You must be signed in to change notification settings - Fork 54
Conversation
It seems that the fix which is merged in #1124 is broken by this pull. The code from that pull is present, which gives me that the develop branch has been merged correctly. When I fill in the title, description and location the job keeps saying that the required blocks aren't filled in. When I fill in all blocks with a random value, thus also the recommended blocks I get this: |
I also wanted to mention that the fix itself seems to work. You only have to fill in all the other fields. When leaving a block to the variation selector I will the job posting as invalid. When selecting a variation the bullet will become green when a value is present. So I think the regression from #1124 needs to be fixed before merging this PR. |
fixed instance names in logging
@increddibelly @hansjovis I have a couple of remarks (this is not a complete review):
|
removed messy piece of innerblocks validation removed unused function
CR:
I also did a smoke acceptance test while reviewing, and found the following:
|
renamed validation result for consistency improved validation for VariationPicker
@increddibelly Code looks good to me. Did you have a chance to look into this comment by Johanna?
I see that you updated the test instructions but did not touch the changelog items. I tend to agree with Johanna's comment. With the added refactoring the PR adds more than validation logic to the variation picker and I think the changelog items should reflect that. |
Acceptance test: ✅ There is still an issue, but we decided to split it off to P2-855. |
Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
Test instructions
Test this PR together with this MonoRepo PR
This PR can be tested by following these steps:
overview
yoast-seo/schema-blocks
store; open thevalidations
.yoast/job-posting
has a score of 303, indicating that a required variation has not been chosen yet.yoast/job-posting
, open theissues
.Block
Instruction andClassname
attribute have a result of 100, indicating they are validated and correct.innerblocks
instruction has a score of 302, indicating a required block is missing.yoast/job-posting
is determined by its worst child result,innerblocks
.innerblocks
and notice it has 10 child results, in the range of :we'll fix these issues one by one and verify that the validation picks up the new changes.
detail
innerblocks
validation result, the first issue should concern thejob-title
. Score should be 301 as it is required but empty.job/salary block
which scores 202, because it is a recommended block but no variation has been picked.job/salary
's score jump to 201. it is a recommended block, but the chosen variation has not been completed.job/salary
block now scores 100, because the chosen variation has been completed and is now valid.repeating until valid
conclusion
Impact check
*
UI changes
Quality assurance
Fixes #