dspace-angular icon indicating copy to clipboard operation
dspace-angular copied to clipboard

Listable-object decorator refactor

Open Atmire-Kristof opened this issue 3 years ago • 1 comments

References

Fixes #1911

Description

This PR refactors listable-object decorator to use a new system for determining the relevancy between matching components and returning the most relevant one for the inputs provided. Previously, the code would ignore some inputs the user provided in certain cases (see issue), this should fix that.

Instructions for Reviewers

Changes made:

  • listable-object decorator has a new internal class called MatchRelevancy. This class stores a match found within the map and information about how it was found. It stores the first "level" a default value was used and the "relevancy" representing the amount of non-default values were used to find this match.
  • Using this class, getListableObjectComponent can determine which of the found matches is the most relevant one.
  • Retrieving a match wrapped within a MatchRelevancy is done through a new getMatch() method, which works roughly the same way it used to, but touches on the issues the previous code had, whilst simultaneously making it open for change (e.g. easy to add a new parameter to our decorator)
  • All of this is well documented within the code, so please check out the JSDocs added in this PR for more details and examples

How to test:

  • The original test cases still succeed, pointing towards me not breaking any existing functionality
  • New test cases have been added that cover my edge-cases from the issue and more
  • These new tests fail when the code is reverted to its original state
  • The UI should be unaffected if you visit different list views, item pages, etc.
  • Optionally, you can take the example from the issue and confirm the right component is resolved after my changes

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

  • [x] My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • [x] My PR passes TSLint validation using yarn run lint
  • [x] My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • [x] My PR passes all specs/tests and includes new/updated specs for any bug fixes, improvements or new features. A few reminders about what constitutes good tests:
    • Include tests for different user types (if behavior differs), including: (1) Anonymous user, (2) Logged in user (non-admin), and (3) Administrator.
    • Include tests for error scenarios, e.g. when errors/warnings should appear (or buttons should be disabled).
    • For bug fixes, include a test that reproduces the bug and proves it is fixed. For clarity, it may be useful to provide the test in a separate commit from the bug fix.
  • [x] If my PR includes new, third-party dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.

Atmire-Kristof avatar Oct 19 '22 12:10 Atmire-Kristof

Just to clarify: I force pushed the three commits into one commit for a cleaner git history

Atmire-Kristof avatar Oct 21 '22 13:10 Atmire-Kristof