Skip to content

Conversation

@rpenido
Copy link
Contributor

@rpenido rpenido commented Nov 22, 2023

Description

This PR adds a new menu item in the Taxonomy Card menu and the Taxonomy Details menu to allow users to import tags to existing Taxonomies by importing a CSV/JSON file. This overwrites the current tags.

import-tags

Supporting Information

Testing instructions

  1. Have an import taxonomy template ready to be imported. You could get one from below:
  2. Go to the taxonomy list page (http://localhost:2001/taxonomies)
  3. Click on the menu from an existing taxonomy card
  4. You will be prompted to select a file to be imported.
  5. Canceling the prompt will abort the operation
  6. If the taxonomy is imported successfully, you will see an alert: Taxonomy imported successfully.
  7. Importing an invalid taxonomy file, you will see an alert: Import failed - see details in the browser console. The error info will be logged in the browser console.

Private-ref: FAL-3536

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Nov 22, 2023
@openedx-webhooks
Copy link

Thanks for the pull request, @rpenido! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

This is currently a draft pull request. When it is ready for our review and all tests are green, click "Ready for Review", or remove "WIP" from the title, as appropriate.

@codecov
Copy link

codecov bot commented Nov 22, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (02cdccc) 88.62% compared to head (c055d56) 88.77%.
Report is 4 commits behind head on master.

❗ Current head c055d56 differs from pull request most recent head 8c2de23. Consider uploading reports for the commit 8c2de23 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #708      +/-   ##
==========================================
+ Coverage   88.62%   88.77%   +0.15%     
==========================================
  Files         446      449       +3     
  Lines        6851     6925      +74     
  Branches     1465     1470       +5     
==========================================
+ Hits         6072     6148      +76     
+ Misses        754      752       -2     
  Partials       25       25              

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

Copy link
Contributor

@pomegranited pomegranited left a comment

Choose a reason for hiding this comment

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

👍 one nit, but LGTM!

  • I tested this on my devstack. There's a known issue with re-importing tags that affects the backend logic, but
  • I read through the code (on open-craft#11)
  • I checked for accessibility issues by using my keyboard to navigate
  • Includes documentation
  • User-facing strings are extracted for translation

*/
/* eslint-disable no-alert */
/* eslint-disable no-console */
console.log(intl);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: remove debugging statements

Suggested change
console.log(intl);

Co-authored-by: Jillian <jill@opencraft.com>
@rpenido
Copy link
Contributor Author

rpenido commented Nov 24, 2023

Closed in favor of #675

@rpenido rpenido closed this Nov 24, 2023
@openedx-webhooks
Copy link

@rpenido Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

1 similar comment
@openedx-webhooks
Copy link

@rpenido Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

@rpenido rpenido deleted the rpenido/fal-3536-bare-bones-update-of-existing-taxonomy-by-upload branch November 24, 2023 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

open-source-contribution PR author is not from Axim or 2U

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[Tagging] "Bare Bones" Update of Existing Taxonomy by Uploading

3 participants