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

beetsplug/web: fix translation of query path #4182

Merged
merged 1 commit into from Jan 6, 2022

Conversation

sumpfralle
Copy link
Contributor

The routing map translator QueryConverter was misconfigured:

  • decoding (parsing a path): splitting with "/" as tokenizer
  • encoding (translating back to a path): joining items with "," as separator

Instead the encoding should have used the same delimiter (the slash) for the backward conversion.

How to reproduce:

  • query: /album/query/albumartist::%5Efoo%24/original_year%2B/year%2B/album%2B
  • resulting content in parsed argument queries in the album_query function:
    • previous (wrong): ['albumartist::^foo$,original_year+,year+,album+']
    • new (correct): ['albumartist::^foo$', 'original_year+', 'year+', 'album+']

I am not sure, why the issue was not noticed before. Maybe werkzeugs handling of routing maps has changed.

@sampsyo
Copy link
Member

sampsyo commented Dec 3, 2021

I think this looks right—I imagine it was just broken before! Thanks for tracking this down.

This seems related to #3567, so perhaps @nmeum would be able to take a look to make sure we're not doing anything wrong?

And in the mean time, @sumpfralle, any chance you could add a quick changelog entry describing the fix?

The routing map translator `QueryConverter` was misconfigured:
* decoding (parsing a path): splitting with "/" as tokenizer
* encoding (translating back to a path): joining items with "," as separator

This caused queries containing more than one condition (separated by a
slash) to return an empty result.  Queries with only a single condition
were not affected.

Instead the encoding should have used the same delimiter (the slash) for the
backward conversion.

How to reproduce:
* query: `/album/query/albumartist::%5Efoo%24/original_year%2B/year%2B/album%2B`
* resulting content in parsed argument `queries` in the `album_query` function:
    * previous (wrong): `['albumartist::^foo$,original_year+,year+,album+']`
    * new (correct): `['albumartist::^foo$', 'original_year+', 'year+', 'album+']`
@sumpfralle
Copy link
Contributor Author

I added a changelog entry.
Thank you for your time!

@sampsyo
Copy link
Member

sampsyo commented Jan 6, 2022

Looks good; thank you! Please feel free to weigh in if we are doing something wrong, @nmeum.

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.

None yet

2 participants