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

Renaming some variables #58

Open
2 of 3 tasks
curquiza opened this issue Feb 25, 2021 · 4 comments
Open
2 of 3 tasks

Renaming some variables #58

curquiza opened this issue Feb 25, 2021 · 4 comments
Labels
good first issue Good for newcomers

Comments

@curquiza
Copy link
Member

curquiza commented Feb 25, 2021

This package was strongly based on the Algolia Symfony Bundler (https://github.com/algolia/search-bundle/ - MIT License).
MeiliSearch and Algolia are indeed similar but not exactly the same. It means there are some naming inconsistencies in the code base of this repository.

I would rather each point be done in different PRs 🙂
Feel free to ask any question you need!

@curquiza curquiza added this to Open issues in SDKs & Integrations via automation Feb 25, 2021
@curquiza curquiza changed the title Renaming Renaming some variables Mar 20, 2021
@skylord123
Copy link

Why are we removing the objectID usage? When objects get placed into Meilisearch the objectID param is set to the primary keys of the entity (or just the single value if there aren't multiple primary keys). When we pull objects down out of MeiliSearch we can just parse the objectID field to figure out how to query for the entity.

I sort of like how it works now because it keeps the config to a minimum (don't have to define the primary key in config) and also doesn't require running two http requests for every search (to figure out the primary key). It would be cool to override the objectID field with your own way if you really want but how it is currently setup is easiest for users to get started with IMO.

I just updated the MeiliSearchService::search function to the following and it works really well (handles multiple primary keys and entities that don't use id as their primary key):

    public function search(
        ObjectManager $objectManager,
        string $className,
        string $query = '',
        array $requestOptions = []
    ): array {
        $this->assertIsSearchable($className);

        $ids = $this->engine->search($query, $this->searchableAs($className), $requestOptions);
        $results = [];

        // Check if the engine returns results in "hits" key
        if (!isset($ids['hits'])) {
            throw new SearchHitsNotFoundException('There is no "hits" key in the search results.');
        }

        foreach ($ids['hits'] as $objectID) {
            if (in_array($className, $this->aggregators, true)) {
                $entityClass = $className::getEntityClassFromObjectID($objectID);
                $id = $className::getEntityIdFromObjectID($objectID);
            } else {
                $multipleIds = explode('__', $objectID['objectID']);
                $entityClass = $className;

                if (0 === count($ids)) {
                    throw new Exception('Entity has no primary key');
                } else if(count($multipleIds) === 1) {
                    $id = $multipleIds[0];
                    $idFieldName = $objectManager->getClassMetadata($entityClass)->getSingleIdentifierFieldName();
                    $repo = $objectManager->getRepository($entityClass);
                    $entity = $repo->findOneBy([$idFieldName => $id]);
                } else {
                    $idFieldNames = $objectManager->getClassMetadata($entityClass)->getIdentifierFieldNames();
                    $searchIdFields = [];
                    foreach($multipleIds as $idPart) {
                        list($idFieldName, $idFieldValue) = explode("-", $idPart);
                        if(!in_array($idFieldName, $idFieldNames)) {
                            continue;
                        }
                        $searchIdFields[$idFieldName] = $idFieldValue;
                    }
                    $repo = $objectManager->getRepository($entityClass);
                    $entity = $repo->findOneBy($searchIdFields);
                }
            }

            $repo = $objectManager->getRepository($entityClass);
            $entity = $repo->findOneBy(['id' => $id]);

            if (null !== $entity) {
                $results[] = $entity;
            }
        }

        return $results;
    }
    ```

bors bot added a commit that referenced this issue Apr 27, 2021
69: Rename indexName to indexUid r=curquiza a=codedge

This PR renames the `indexName` variables to `indexUid`, see #58.

Co-authored-by: Holger Lösken <[email protected]>
@bors bors bot closed this as completed in f3f9db4 Apr 27, 2021
@curquiza curquiza reopened this Apr 27, 2021
bors bot added a commit that referenced this issue Apr 28, 2021
70: Rename requestOptions to searchParams r=curquiza a=codedge

This PR renames the `requestOptions` variable to `searchParams`, see #58.

Co-authored-by: Holger Lösken <[email protected]>
Co-authored-by: Holger Lösken <[email protected]>
@codedge
Copy link
Contributor

codedge commented Apr 30, 2021

So the second tasks is fixed as well (#70). The objectId stuff is done in #66 - I would also start taking care of that but first let's crush #76 (little reminder @curquiza ) 😉

@curquiza
Copy link
Member Author

curquiza commented Jun 9, 2021

I let this issue opened since there is still some objectID in the code, but you did most of the job, thank you very much 😁

@codedge
Copy link
Contributor

codedge commented Jun 9, 2021

Yes, for the objectId I would create a new branch & PR to work on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
No open projects
Development

No branches or pull requests

4 participants