Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid empty results Array in case of no mismatches #171

Merged
merged 1 commit into from Oct 26, 2021

Conversation

Silvan-WMDE
Copy link
Member

This change makes sure that instead of sending an empty array in the
Inertia response, no results field is passed to Results.vue, when no
mismatches have been found. This way the results prop's default
function will be executed to create an empty object, which prevents an
'Invalid prop' error message, previously caused by the empty array.

Bug: T294303

This change makes sure that instead of sending an empty array in the
Inertia response, no results field is passed to Results.vue, when no
mismatches have been found. This way the results prop's default
function will be executed to create an empty object, which prevents an
'Invalid prop' error message, previously caused by the empty array.

Bug: T294303
@Silvan-WMDE Silvan-WMDE force-pushed the fix/prevent-empty-results-array branch from 1bb3f59 to fe4aacf Compare October 25, 2021 21:07
Copy link
Member

@itamargiv itamargiv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, no console error 👍 Thank you!

'labels' => $wikidata->getLabels($entityIds, App::getLocale())
],
// only add 'results' prop if mismatches have been found
$mismatches->isNotEmpty() ? [ 'results' => $mismatches->groupBy('item_id') ] : []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful! Elegant! 👏

->assertViewIs('app')
->assertInertia(function (Assert $page) {
$page->component('Results')
->missing('results');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Our tests even reflected the issue before, which is also good!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha, I guess that's your nice way of saying "our tests were wrong". :-)

@Silvan-WMDE Silvan-WMDE merged commit 9336278 into main Oct 26, 2021
@Silvan-WMDE Silvan-WMDE deleted the fix/prevent-empty-results-array branch October 26, 2021 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants