Refacto rename indexName to indexUid #89
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
this PR corrects the minor omnission of PR # 69
This does not work correctly with a UUID or ULID object as the primary key. To index a document, no problem, Meilisearch does convert the ObjectId into a character string. On the other hand, to delete a resource from MeiliSearch, you must indicate the ObjectId in character string in the url of the DELETE request. While waiting for the refactoring on the objectID which will surely correct this bug (# 58). My fix while waiting for the refacto of the objectId is to indicate the ObjectID property of the normalized object instead of entity-> getID ().
|
It seems to be the continuity of your previous job @codedge, I ask you for a review if you have time 🙂 If you don't, tell me. |
|
Yes, will do! |
codedge
reviewed
Jun 11, 2021
|
Build succeeded: |
bors bot
added a commit
that referenced
this pull request
Jun 22, 2021
92: Allowing different entities indexed into same index r=curquiza a=codedge While this PR got out of control 😄 and poses a bigger rewrite of the tests, it addresses multiple issues. ## Passing index names via command line Passing index name via command line now takes care if the name of the is already prefixed or not. Running - `bin/console meili:import --indices=prefix_indexName` - `bin/console meili:import --indices=indexName` leads to the same index creations which is `prefix_indexName` - given the `prefix` hold a value. ## Indexing different models into the same index See #90 Although MS supports importing different models into the same index, this was not easiliy possible using this package. The configuration in `meili_search.yml` would not allowe to set the same index name twice. Now you can do the following ```yaml meili_search: # Other... indices: - name: tags class: 'MeiliSearch\Bundle\Test\Entity\Tag' index_if: isPublic # Yes, we want to have links in the same index as tags # We just set the same index name 'tags' - name: tags class: 'MeiliSearch\Bundle\Test\Entity\Link' index_if: isSponsored ``` This would lead to the result of having models of `Tag` and `Link` inside the very same index `tags`.⚠️ Make sure your identity key is unique across both models. Given you use a field `id`, the value of this needs to be unique across both models. @curquiza When the PR is approved and merged this should be documented inside a the wiki. ## Using aggregators See #90 You can now create an aggregator, register your models and will get an aggregated index created in MS. As the identity field of each model is used as identity field in the aggregated index... > Make sure your identity key is unique across both models. Given you use a field `id`, the value of this needs to be unique across both models. @curquiza When the PR is approved and merged this should be documented inside a the wiki. ## Fixing inconsistency See #89 Fixes some left of overs from refatoring `indexName` -> ìndexUid`. #89 is outdated with this PR here. ## Further changes - No separate folder to hold the test database in `tests/blog.sqlite`. Just use the `var/test.sqlite` folder which already holds cached/runtime data. - Reset the database and MS instance for each test via the `setUp` method. No need to have a dedicated `refreshDb` and `clearUp` call spread across the test. Each test should be atomic and not relying on former data. So 🗑️ everything 😃 - Update the `phpunit.xml` to the more recent format using the [`--migrate-configuration`](https://phpunit.readthedocs.io/en/9.5/textui.html?highlight=migrate#command-line-options) option. - Updating the test entities to make it easier with having properties inside the database and imported into MS. - Converting the config to a `Collection` object, see `illuminate/collection`. Makes it a lot easier to deal with, if you ask me 🙈 Co-authored-by: Holger Lösken <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
this PR corrects the minor omnission of PR #69