Skip to content

FIX Use search context when no database field for label#11655

Merged
GuySartorelli merged 1 commit intosilverstripe:5.3from
creative-commoners:pulls/5.3/search-fallback
Mar 30, 2025
Merged

FIX Use search context when no database field for label#11655
GuySartorelli merged 1 commit intosilverstripe:5.3from
creative-commoners:pulls/5.3/search-fallback

Conversation

@emteknetnz
Copy link
Copy Markdown
Member

Issue #11647

@emteknetnz emteknetnz marked this pull request as ready for review March 27, 2025 05:16
@emteknetnz emteknetnz force-pushed the pulls/5.3/search-fallback branch from df86694 to 785855a Compare March 27, 2025 05:24
Copy link
Copy Markdown
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

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

The PHPDoc for getUseSearchContext() and setUseSearchContext() should be updated to reflect that the search context will be used if the label field isn't a database field even if useSearchContext is false.

The fact that label field is used for searching by default should maybe also be added to setLabelField() and getLabelField().

Alternatively this context could maybe be added to the PHPDoc for the class itself.

Comment thread tests/php/Forms/SearchableDropdownTraitTest.php
@emteknetnz emteknetnz force-pushed the pulls/5.3/search-fallback branch from 785855a to 213954d Compare March 27, 2025 21:24
@emteknetnz
Copy link
Copy Markdown
Member Author

Have added docblock comments

Comment thread src/Forms/SearchableDropdownTrait.php Outdated
@emteknetnz emteknetnz force-pushed the pulls/5.3/search-fallback branch from 213954d to ef6c4cf Compare March 27, 2025 21:56
@lekoala
Copy link
Copy Markdown
Contributor

lekoala commented Mar 28, 2025

:-) this is looking great thanks for the follow up!

Copy link
Copy Markdown
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

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

LGTM, works as expected

@GuySartorelli GuySartorelli merged commit a9914bb into silverstripe:5.3 Mar 30, 2025
17 checks passed
@GuySartorelli GuySartorelli deleted the pulls/5.3/search-fallback branch March 30, 2025 20:18
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.

3 participants