Skip to content

SOLR-16142: Fix Admin UI's spatial parameter generation.#886

Merged
cpoerschke merged 5 commits into
apache:mainfrom
cpoerschke:SOLR-16142
Jun 28, 2022
Merged

SOLR-16142: Fix Admin UI's spatial parameter generation.#886
cpoerschke merged 5 commits into
apache:mainfrom
cpoerschke:SOLR-16142

Conversation

@cpoerschke
Copy link
Copy Markdown
Contributor

@cpoerschke cpoerschke commented Jun 1, 2022

@cpoerschke cpoerschke requested review from epugh and janhoy June 1, 2022 16:37
@cpoerschke
Copy link
Copy Markdown
Contributor Author

cpoerschke commented Jun 1, 2022

Perhaps last minute candidate for 8.11.2 if this fix is accurate? cc/fyi @madrob

@cpoerschke cpoerschke marked this pull request as ready for review June 1, 2022 16:38
epugh
epugh approved these changes Jun 1, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@epugh epugh left a comment

Choose a reason for hiding this comment

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

LGTM

@cpoerschke
Copy link
Copy Markdown
Contributor Author

cpoerschke commented Jun 1, 2022

Hmm, I'm not very familiar with this code but from trying to learn via the #122 diff there is I see at https://github.com/apache/solr/blob/releases/solr/9.0.0/solr/webapp/web/js/angular/controllers/query.js#L145 some getDependentFields("spatial") logic which might break if the spatial. prefix is simply dropped? So change/fix might be a little more involved?

@epugh
Copy link
Copy Markdown
Contributor

epugh commented Jun 1, 2022

okay, digging in some more

@cpoerschke
Copy link
Copy Markdown
Contributor Author

cpoerschke commented Jun 1, 2022

... getDependentFields("spatial")logic which might break if thespatial.` prefix is simply dropped? ...

okay, so experimentally, when the spatial checkbox is unchecked then it continues to send the spatial parameters and presumably it's not meant to do that.

@cpoerschke
Copy link
Copy Markdown
Contributor Author

cpoerschke commented Jun 1, 2022

okay, digging in some more

so amending the purgeParams calls works I think, making the spatial checkbox similar to the dismax and edismax dropdown options. WDYT?

@cpoerschke cpoerschke requested a review from epugh June 1, 2022 17:22
@cpoerschke cpoerschke merged commit 0bd8c4d into apache:main Jun 28, 2022
@cpoerschke cpoerschke deleted the SOLR-16142 branch June 28, 2022 17:06
asfgit pushed a commit that referenced this pull request Jun 28, 2022
@cpoerschke
SOLR-16142: Fix Admin UI's spatial parameter generation. (#886)
d89d936 
(cherry picked from commit 0bd8c4d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@janhoy janhoy Awaiting requested review from janhoy

@epugh epugh Awaiting requested review from epugh

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cpoerschke @epugh