Handle multiple index names in strapi config #916
Conversation
we have to use restaurant as content type to really testthis function
this will allow to link a content type to multiples indexes BREAKING CHANGE: in strapi config indexName will have to be an array
… array index name BREAKING CHANGE: indexName needs to be an array in strapi config
to handle indexName array
to handle array index name
to handle multiple index name
to handle multiple indexes
to handle multiple indexes
to handle multiple indexes
to handle multiple indexes we will have one row per index name
to handle multiples index names
…exNames Feat/index name/handle multiple index names
instead of a string
feat(configuration-validation): test that indexName is an array
brunoocasali
added
the
breaking-change
There was a problem hiding this comment.
Hi @L-Weisz!
What a high-quality PR! Thanks a lot for your effort here!
I would like to discuss some of the decisions I made from the diff. Let me know what you think about them.
Apart from them, I want to know if you considered a way to avoid the breaking change?
| @@ -113,7 +113,7 @@ describe('Test plugin configuration', () => { | |||
| expect(strapiMock.log.error).toHaveBeenCalledTimes(0) | |||
| }) | |||
|
|
|||
| test('Test indexName with empty string', async () => { | |||
| contentType, | ||
| entryId: entry.id, | ||
| const indexUids = config.getIndexNamesOfContentType({ contentType }) | ||
| await Promise.all( |
There was a problem hiding this comment.
Should we use allSettled here?
There was a problem hiding this comment.
And in the other places where we make calls for all the indexes...
There was a problem hiding this comment.
It depends on how we want to handle the rejection of the promises generated by
contentTypeConfig.transformEntry.
Use Promise.all if: You want the entire operation to fail as soon as one of the transformEntry calls fails. This is useful when it's critical that all entries must be successfully transformed; if any single transformation fails, the whole batch should be considered failed, and you might want to handle that error specifically, perhaps by logging or retrying.
Use Promise.allSettled if: You want to wait for all promises to settle, regardless of whether they are fulfilled or rejected. This approach is useful when you want to attempt to transform all entries and then deal with them individually, depending on whether each promise was fulfilled or rejected. It allows you to handle each entry's transformation success or failure case separately.
which one do you think we should use ?
There was a problem hiding this comment.
If we had a way to retry the failed entries would be cool I guess. But we can do it later :)
… an array of tasks
to avoid breaking change
Feat/index names
|
Hi @brunoocasali thanks a lot for your feedback! Really appreciated your insights I’ve pushed some updates to reflect your suggestions. On the topic of the breaking change, I found the issue was with how the validator treated the indexName. I made sure it now accepts indexName as a string, avoiding any breaking changes ! 🎉 (As the function that retrieves indexNames already transforms a simple string into an array) |
|
Hello @brunoocasali were you able to check the changes ? |
|
Hello here @brunoocasali is still on Holidays and check when he'll come back |
|
Thanks a lot for your feedback @brunoocasali ! so happy to contribute |
brunoocasali
removed
the
breaking-change
Pull Request
Related issue
Fixes #651
What does this PR do?
This Pull Request introduces the capability to associate a single collection with multiple index names within Strapi's configuration. Previously, the indexName parameter was limited to a single string value, specifying one index per collection. Now, indexName can be defined as an array, enabling a collection to be indexed under multiple names simultaneously.
For example, the configuration can now be specified as:
Before: indexName: "my_restaurant"
After: indexName: "my_restaurant" (after review, found a way to avoid the breaking change)
After: indexName: ["my_restaurant"]
After: indexName: ["my_restaurant", "all_restaurants"]
Additionally, this update includes the introduction of new tests to ensure that the expanded functionality does not disrupt existing features. An existing test was also corrected.
Important Changes:
Limitation: While indexName can now accept multiple values, all other configuration attributes remain uniform across the specified indexes. This means that any update to one index will apply identically to all indexes associated with a given collection, reflecting the same settings and data.
This enhancement provides greater flexibility in managing index names for collections within Strapi, streamlining the process of data organization and retrieval across multiple indexes.
Result
PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!