Skip to content

Avoid duplicate column suggestions in search bar #556

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

Closed
wants to merge 1 commit into from

Conversation

raviks789
Copy link
Contributor

@raviks789 raviks789 commented Jun 21, 2022

ObjectSuggestions::collectFilterColumns() returns duplicate column names due to the sub-relations of the joined models. Hence, ObjectSuggestions::fetchColumnSuggestions() returns duplicate suggestions for column names for the search bar. This fix solves this issue by resolving the redefinition of the method parameter $model in ObjectSuggestions::collectFilterColumns() and not registering the duplicate models in ObjectSuggestions::collectRelations().

fix #534

@raviks789 raviks789 requested a review from nilmerg June 21, 2022 14:28
@cla-bot cla-bot bot added the cla/signed CLA is signed by all contributors of a PR label Jun 21, 2022
@raviks789 raviks789 force-pushed the fix/avoid-duplicate-column-suggestion branch from 81f069d to 168bcc8 Compare June 21, 2022 14:34
@lippserd lippserd requested a review from yhabteab June 24, 2022 09:30
@raviks789 raviks789 force-pushed the fix/avoid-duplicate-column-suggestion branch from 168bcc8 to e38a591 Compare June 24, 2022 10:13
@raviks789 raviks789 changed the title Avoid duplicate column suggestion in Search bar. Avoid duplicate column suggestions in search bar. Jun 24, 2022
@raviks789 raviks789 force-pushed the fix/avoid-duplicate-column-suggestion branch 2 times, most recently from 110f51e to d24ce21 Compare June 24, 2022 13:04
@raviks789 raviks789 changed the title Avoid duplicate column suggestions in search bar. Avoid duplicate column suggestions in search bar Jun 24, 2022
Copy link
Member

@nilmerg nilmerg left a comment

Choose a reason for hiding this comment

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

A key feature of this procedure is to only produce as much results as necessary. This change produces all columns of all relations at once, making it more inefficient.

Please think of a different solution, maybe along the lines of filtering out duplicate relation targets in the sub-procedures.

@nilmerg nilmerg added the bug Something isn't working label Jun 27, 2022
`ObjectSuggestions::collectFilterColumns()` returns duplicate column names due to the sub-relations of the joined models.
Hence, `ObjectSuggestions::fetchColumnSuggestions()` returns duplicate suggestions for column names for the search bar.
This fix solves this issue by resolving the redefinition of the method parameter `$model` in `ObjectSuggestions::collectFilterColumns()`
and not registering the duplicate models in `ObjectSuggestions::collectRelations()`.
@raviks789 raviks789 force-pushed the fix/avoid-duplicate-column-suggestion branch from d24ce21 to 335c08d Compare June 28, 2022 09:53
@raviks789 raviks789 requested a review from nilmerg June 28, 2022 09:55
nilmerg pushed a commit that referenced this pull request Jun 29, 2022
Also a fixes that some columns are shown repeatedly.

fixes #571
refs #556
nilmerg pushed a commit that referenced this pull request Jun 29, 2022
Also a fixes that some columns are shown repeatedly.

fixes #571
refs #556
@nilmerg
Copy link
Member

nilmerg commented Jul 18, 2022

The variable name fix has been cherry picked (7368f87). The other fix is obsolete since 8f462c6. Closing.

@nilmerg nilmerg closed this Jul 18, 2022
@nilmerg nilmerg deleted the fix/avoid-duplicate-column-suggestion branch July 18, 2022 07:56
@nilmerg nilmerg removed request for nilmerg and yhabteab July 18, 2022 07:56
@nilmerg nilmerg removed the bug Something isn't working label Jul 18, 2022
flourish86 pushed a commit that referenced this pull request Aug 15, 2022
Also a fixes that some columns are shown repeatedly.

fixes #571
refs #556
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla/signed CLA is signed by all contributors of a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some Filter columns are proposed twice
2 participants