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

Fix: bad strict comparison String/Number #351

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

krzysztofkaszanek
Copy link

@krzysztofkaszanek krzysztofkaszanek commented Jun 23, 2023

Ticket

#350

Summary

What does this PR do/solve?

This PR fix the problem of entity not being selected correctly in the item modal because formik use a string named "related" but compared of Number "id".

I believe the same problem may still occur in other places, there was almost identical PR merged recently: #344.

Maybe it would be a good idea to parse the content type item id to String as soon as it is fetched. That would require adding

contentTypeItems: (isArray ? contentTypeItems : [contentTypeItems]).map(item => ({
        ...item,
        id: String(item.id), // add parsing ID to string
        __collectionUid: get(fetchedContentType, 'collectionUid', modelUID),
      })),

in https://github.com/VirtusLab-Open-Source/strapi-plugin-navigation/blob/master/admin/src/pages/DataManagerProvider/index.js#L219.

I don't know the plugin code well enough, and I'm so I decided to add just the "local" fix for now.

Test Plan

@strapi/strapi 4.11.2
strapi-plugin-navigation 2.2.11

  1. Create collection type Page with field title of type Text
  2. Set title in Name fields in navigation plugin settings
  3. Add new menu item by selecting existing Page entity
  4. Menu item is created with the title of Page entity (this was not the case before the fix, nothing was happening when you click Save)

@@ -139,7 +139,7 @@ const NavigationItemForm: React.FC<NavigationItemFormProps> = ({
}
} else {
selectedEntity = {
...contentTypeEntities.find(_ => _.id === related),
...contentTypeEntities.find(_ => String(_.id) === related),
Copy link
Contributor

Choose a reason for hiding this comment

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

_.id.toString() === related.toString()

to be sure about the type of both

@SalahAdDin
Copy link

@krzysztofkaszanek Are you currently working on Strapi?

@SalahAdDin
Copy link

@cyp3rius since the ticket was solved, I guess you could close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants