Added upper limit to browse by date (year)

[mspalti]

Description

Attempts to set an upper limit to the browse by year options. So for example if a collection contains publications from 2001- 2010 the first year in the dropdown list is 2010 (not 2023).

List of changes in this PR:

• Modified the browse-by-date-page.component to set the upper limit by checking the date field of the most recent item.
• Modified the browse.service function to accept an optional SortDirection parameter.

To test this, browse a collection in which no item has a date.issued value from 2023. The first year in the pulldown should match the most recent item in the collection. Note that if the most recent item doesn't have a properly formatted date then the year option will default to the current (2023) year.

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!

• 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.
• My PR passes ESLint validation using yarn lint
• My PR doesn't introduce circular dependencies (verified via yarn check-circ-deps)
• My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
• My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
• If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
• If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
• If my PR fixes an issue ticket, I've linked them together.

[mspalti]

@tdonohue, fyi, the test failure for this PR appeared after I rebased main. I also see the same error intermittently when I build and test my copy of the main branch.

Besides the test, the metadata browse component UI isn't working for main. The "back to results" button updates the page with items (not metadata). That actually aligns with the test failure. But I don't see the same UI problem on the demo site, which is odd.

I think the problem is related to #2139, which was recently merged. I think given how the browse-by-metadata component works it's necessary to always set the value property in the subscription rather than reset it conditionally when typeof params.value === 'string' as was done in that PR. That approach means it isn't reset when the params.value changes to undefined.

If that's the case then I suppose the tests might fail intermittently due to execution order.

I went ahead and reverted the one line change in browse-by-metadata-page.component.ts. Noting that here in case it needs discussion. Looks like the change wasn't essential for that PR since it's focused on the startsWith parameter, but I could be missing something.

[mspalti]

@artlowel , I just verified a bug in main that was introduced in #2139. It popped up as a random test error when I was working on this PR. The problem is that the "all browse results" back button no longer works because of this change line:

dspace-angular/src/app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component.ts

Line 155 in 1ddeeed

if (typeof params.value === 'string'){

It's inside a subscription and no longer updates when params.value emits as undefined. I think that's causing the back button to return to a list of items instead of metadata values. Do you agree? If so, I've gone ahead and reverted that change in this PR.

[tdonohue]

@mspalti : I believe you are correct that there is a bug caused by #2139. However, reverting the change in #2139 seems to re-introduce some of the problems described in #2128 (solved by #2139).

So, I wonder if the proper code here should be something more like this?

if (typeof params.value === 'string'){
    this.value = params.value.trim();
} else {
    this.value = '';
}

The problems with the old logic are described in #2128 (the old +params.value trims off leading zeros which can mess up the search or make it look odd)... so I don't think we can just revert. But, I do agree with you that it appears we introduced a bug. I'm not yet certain of the proper solution, but wanted to point out that reverting may not be the best solution.

[tdonohue]

@mspalti : Tested this today and it works as described, so I'm basically a +1. I think you are correct that you've found a bug in browse-by-metadata-page updates... but, I'm not sure reverting it is the correct fix. See comment above and my inline comment below.

[mspalti]

@tdonohue, setting value = '' as the fallback does indeed fix the bug. I've made the addition.

[tdonohue]

Thanks @mspalti ! I'll retest quickly and then get this merged.

[tdonohue]

👍 Thanks @mspalti ! This looks good. Also looks to fix the small bug introduced by #2139. Merging immediately.