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

Refactor Bulk Tagger client-side code #8444

Conversation

jimchamp
Copy link
Collaborator

@jimchamp jimchamp commented Oct 19, 2023

Refactors some of the Bulk Tagger code. Changes:

  • Client-side Bulk Tagger code has been moved to /js/ile
  • BulkTagger class created to encapsulate client-side Bulk Tagger code
  • Change form's hidden inputs to use type="hidden" (as opposed to hiding them via CSS)
  • Prevent fetching subject suggestions if the search term is an empty string
  • Use hidden class to hide the Bulk Tagger
  • Remove ?debug=true from the bulk tagging form's action
  • Use correct tag_subjects default in handler
  • Replace unused id in "Tag selections" anchor with an href

Technical

Testing

Confirm that the bulk tagger is working as expected.

Screenshot

Stakeholders

@mekarpeles

@jimchamp jimchamp force-pushed the 8433/bug/async-bulk-tagger-submissions branch from e954e34 to 37dabdf Compare October 19, 2023 23:21
@codecov-commenter
Copy link

codecov-commenter commented Oct 19, 2023

Codecov Report

Attention: Patch coverage is 2.32558% with 126 lines in your changes missing coverage. Please review.

Project coverage is 16.03%. Comparing base (1ac93f7) to head (01df0f5).
Report is 1656 commits behind head on master.

Files with missing lines Patch % Lines
...enlibrary/plugins/openlibrary/js/ile/BulkTagger.js 2.43% 111 Missing and 9 partials ⚠️
...ibrary/plugins/openlibrary/js/ile/utils/subject.js 0.00% 5 Missing ⚠️
.../js/ile/utils/SelectionManager/SelectionManager.js 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8444      +/-   ##
==========================================
+ Coverage   15.97%   16.03%   +0.06%     
==========================================
  Files          86       86              
  Lines        4695     4677      -18     
  Branches      822      816       -6     
==========================================
  Hits          750      750              
+ Misses       3422     3406      -16     
+ Partials      523      521       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mekarpeles mekarpeles self-assigned this Oct 23, 2023
@jimchamp jimchamp marked this pull request as draft October 23, 2023 19:51
@jimchamp jimchamp changed the title Prevent Bulk Tagger from redirecting on batch update [WIP] Refactor Bulk Tagger client-side code Oct 23, 2023
@jimchamp jimchamp force-pushed the 8433/bug/async-bulk-tagger-submissions branch from ec94ca3 to 01df0f5 Compare November 1, 2023 16:03
@jimchamp jimchamp changed the title [WIP] Refactor Bulk Tagger client-side code Refactor Bulk Tagger client-side code Nov 1, 2023
@jimchamp jimchamp marked this pull request as ready for review November 1, 2023 19:44
@mekarpeles mekarpeles merged commit 3d1abad into internetarchive:master Nov 9, 2023
3 checks passed
@jimchamp jimchamp deleted the 8433/bug/async-bulk-tagger-submissions branch November 16, 2023 17:19
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