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

Feature/dynamic thesauri fixed #549

Merged
merged 7 commits into from
Jul 10, 2023
Merged

Conversation

jwaspin
Copy link
Contributor

@jwaspin jwaspin commented Jun 6, 2023

Closing issues

closes #546

Pull Request

  • Please check if the PR fulfills these requirements

    • The commit message follows our guidelines
    • Tests for the changes have been added (for bug fixes / features)
    • Docs have been added / updated (for bug fixes / features)
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    Feature

  • What is the current behavior? (You can also link to an open issue here)
    Add dynamic thesauri from config files #546

  • What is the new behavior (if this is a feature change)?
    Add dynamic thesauri from config files #546

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    No, this should not break anything. Worst case the user will need to refresh their browser for the updated application.

  • Other information:
    This PR includes changes to the package.json file:

  • axios was added to handle retrieving the config files

  • gcmd-keywords and mdKeywords modules have been removed

Fixed the integration test associated with the thesaurus select component. It was looking for the id of the first thesaurus added, which was changed, so the id was updated to reflect the thesaurus id that is now the first one loaded.

Testing

Deployed at http://34.201.136.147:8002

Added axios as a dependency.
Also updating yarn.lock.
Deprecated/removed the gcmd-keywords and mdKeywords modules.
Loading the list of vocabularies dynamically from the config file in
mdKeywords repository.
Using the urls from that config file, loading the keywords dynamically
from other config files in the mdKeywords repository.
Note that the tooltipPath value in the template file needs to be the
same as the name of the tooltipText attribute in the component file.
The text inside the tooltip comes from the "description" attribute in
the citation object for the thesaurus.
Removing gcmd-keywords and mdKeywords from yarn.lock.
Changed the name of the vocabularies file in mdKeywords. Needed to also
be updated in this environment config.
@jwaspin jwaspin requested a review from mikegiddens June 16, 2023 17:26
@hmaier-fws
Copy link
Contributor

@jwaspin There are a few problems with this update.

Throws error when editing existing GCMD thesaurus

  1. Load record containing existing vocabularies
  2. Select "Edit Keywords" for an existing GCMD vocabulary
    Application throws error: "Cannot read properties of undefined (reading 'dynamicLoad')". The ISO and custom thesauri can be edited without a problem.

The expected behavior is to allow the addition or deletion of keywords from the list.

Clear storage cache warning can not be cleared

  1. Goto "Settings" and select "Clear Storage Cache"
  2. The "Are you sure" dialog appears.
  3. Attempt to select "Cancel" or "X" to close dialog box
  4. Attempt to select "Confirm"

There is no way to back out of the operation nor does selecting "Conform" perform the operation.

Expected behavior is to either allow the user to cancel the operation or to confirm and complete the operation. A similar issue has been reported in the past (See: #379)

@hmaier-fws hmaier-fws marked this pull request as draft June 30, 2023 16:21
@jwaspin
Copy link
Contributor Author

jwaspin commented Jun 30, 2023

@hmaier-fws
I fixed the identifiers that were causing the problem. That change was made in mdKeywords to /resources/vocabularies.json.

The issue with 'Clear Storage Cache' is already in the develop branch. It's not introduced by this PR, so we'll have to do a bug fix and another PR for that one.

@jwaspin jwaspin marked this pull request as ready for review July 5, 2023 16:06
@hmaier-fws
Copy link
Contributor

Thesaurus error has been resolved. The "clear storage cache" issue doesn't look like it was introduced in this PR. I'll create a separate issue for that.

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.

Add dynamic thesauri from config files
3 participants