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

Catalog filtering by multiple languages #841

Merged
merged 4 commits into from
Nov 1, 2022

Conversation

veloman-yunkan
Copy link
Collaborator

Fixes kiwix/kiwix-tools#358

The requested functionality is implemented in a slightly different way. The OPDS query was assumed to be a conjunction (AND) of the filtering criteria. Handling multiple lang parameters as a disjunction (OR) may lead to confusion. Instead the set of desired languages must be specified as a comma-separated list via a single lang parameter (similar to how the tag parameter works).

The enhancement applies to 3 endpoints:

  • /catalog/search
  • /catalog/v2/entries
  • /catalog/v2/partial_entries

Note, however, that corresponding unit-tests have been provided only for the first two of them. Filtering of /catalog/v2/partial_entries was not tested (when that endpoint was introduced) because it shares the same code with /catalog/v2/entries.

@codecov
Copy link

codecov bot commented Oct 31, 2022

Codecov Report

Base: 71.16% // Head: 71.20% // Increases project coverage by +0.03% 🎉

Coverage data is based on head (7765769) compared to base (a52138e).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #841      +/-   ##
==========================================
+ Coverage   71.16%   71.20%   +0.03%     
==========================================
  Files          53       53              
  Lines        3711     3716       +5     
  Branches     2060     2062       +2     
==========================================
+ Hits         2641     2646       +5     
  Misses       1068     1068              
  Partials        2        2              
Impacted Files Coverage Δ
include/library.h 100.00% <ø> (ø)
src/server/internalServer.cpp 88.92% <ø> (-0.06%) ⬇️
src/library.cpp 83.82% <100.00%> (+0.30%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@veloman-yunkan
Copy link
Collaborator Author

It turned out /catalog/v2 was not documented at https://wiki.kiwix.org/wiki/OPDS. I updated that page, but after this PR is merged it must be updated again.

Copy link
Member

@mgautierfr mgautierfr left a comment

Choose a reason for hiding this comment

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

LGTM

From now on, the `lang` parameter of the /catalog/search,
/catalog/v2/entries, and /catalog/v2/partial_entries endpoints is
interpreted as a comma-separated list of languages.
@mgautierfr mgautierfr force-pushed the catalog_filtering_by_multiple_languages branch from 2fce69c to 7765769 Compare November 1, 2022 18:16
@mgautierfr mgautierfr merged commit bf9aeff into master Nov 1, 2022
@mgautierfr mgautierfr deleted the catalog_filtering_by_multiple_languages branch November 1, 2022 18:23
@kelson42
Copy link
Collaborator

kelson42 commented Nov 1, 2022

@veloman-yunkan Thank your for this PR. Please don't forget to update the wiki (again)

@veloman-yunkan
Copy link
Collaborator Author

Done

mgautierfr added a commit that referenced this pull request Nov 30, 2022
* [API Break] Remove wrapper around libzim (@mgautierfr #789)
* Allow kiwix-serve to use custom resource files (@veloman-yunkan #779)
* Properly handle searchProtocolPrefix when rendering search result (@veloman-yunkan #823)
* Prevent search on multi language content (@veloman-yunkan #838)
* Use new `zim::Archive::getMediaCount` from libzim (@mgautierfr #836)
* Catalog:
 - Include tags in free text catalog search (@veloman-yunkan #802)
 - Illustration's url is based on book's uuid (@veloman-yunkan #804)
 - Cleanup of the opds-dumper (@veloman-yunkan #829)
 - Allow filtering of catalog content using multiple languages (@veloman-yunkan #841)
 - Make opds-dumper respect the namemapper (@mgautierfr #837)
* Server:
 - Correctly handle `\` in suggestion json generation (@veloman-yunkan #843)
 - Better http caching (@veloman-yunkan #833)
 - Make `/suggest` endpoint thread-safe (@veloman-yunkan #834)
 - Better redirection of main page (@veloman-yunkan #827)
 - Remove jquery (@mgautierfr @juuz0 #796)
 - Better Viewer of zim content :
   . Introduce `/content` endpoints (@veloman-yunkan #806)
   . Switch to iframe based content viewer (@veloman-yunkan #716)
 - Optimised design of the welcome page:
   . Alignement (@juuz0 @kelson42 #786)
   . Exit download modal on pressing escape key (@Juzz0 #800)
   . Add favicon for different devices (@Juzz0 #805)
   . Fix auto hidding of the toolbar (@veloman-yunkan #821)
   . Allow user to filter books by tags in the front page (@juuz0 #711)
* CI :
  - Trigger CI on pull_request (@kelson42 #791)
  - Drop Ubuntu Impish packaging (@legoktm #825)
  - Add Ubuntu Kinetic packaging (@legoktm #801)
* Testing:
  - Test ICULanguageInfo (@veloman-yunkan #795)
  - Introduce fake `test` language to test i18n (@veloman-yunkan #848)
* Fix documentation (@kelson42 #816)
* Udpate translation (#787 #839 #847)
@mgautierfr mgautierfr mentioned this pull request Nov 30, 2022
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.

Allow multiple lang query paramters on OPDS endpoint
3 participants