Fix infinite loading on item pages and optimize menu resolver usage

[alexandrevryghem]

References

• Fixes Infinite Load Times When Editing an Item or Browsing DSpace #3584
• Fixes Item page sometimes fails to load #3425

Description

Fixed the issue where switching between item pages could cause an infinite loading screen. This issue was caused by requests made in dsoEditMenuResolver that set the useCachedVersionIfAvailable to false. The problem arose due to a change in the timing of when a request is actually sent to the backend. Since Angular 16, an asapScheduler causes requests to sometimes happen sooner. This broke this logic, which prevents cached data (when the cache is disabled) and stale data from being returned immediately when the service call is made.

The bitstream and relationship pages also had an additional issue that could cause an infinite loading screen. When navigating to these pages, you could sometimes see the error no elements in sequence in the console. This occurred when taking the first value of an observable that does not emit. This issue has been fixed.

Finally, the menu resolver was being resolved in too many places. It should only be resolved on pages that use the DsoEditMenuComponent. Therefore, it has been restricted to resolve only for the simple item page, the full item page, and the community and collection pages. All other places, such as the edit item/collection/community tabs and the browse tab, no longer need to resolve it. As a result, the edit item metadata tab, for example, now requires five fewer requests.

Instructions for Reviewers

List of changes in this PR:

• Fixed the edge case where sending a request with useCachedVersionIfAvailable === false could cause an infinite loading screen:
  Previously, when a cached version was already present in the store, the RemoteDataBuildService#buildList would always first emit the cached value. This was then followed by a ResponsePending, which in turn was followed by the new response.
  Because we don't want to return the old cached value when useCachedVersionIfAvailable is false, a skipWhile was added to filter out all completed responses until a non-completed response was emitted.

  Due to recent changes, requests are sent earlier, which can cause the RemoteDataBuildService#buildList to already have the new completed response ready. The issue is that the skipWhile mistakenly interprets this as the old cached value (because no non-completed response was emitted), causing the observable to never emit.

  The fix is to update the skipWhile logic to not rely on the RemoteData's state to skip responses. Instead, it should compare the lastUpdated timestamp with the time before the request was sent. This ensures that all responses emitted by the RemoteDataBuildService#buildList are skipped unless they are new.

• Fixed the no elements in sequence issue for the ItemBitstreamsComponent and ItemRelationshipsComponent. This was resolved by replacing the first() operators with take(1) to prevent such an error from being thrown.

• Removed the dsoEditMenuResolver from the browse routes since the browse pages were separated from the community and collection pages in Refactored community & collection pages #2722, making the menu no longer necessary.

• Moved the dsoEditMenuResolver to resolve only for simple/full item pages rather than for every item, collection, or community route. This prevents routes like edit metadata from resolving the menu.

• Updated the correction types call in DSOEditMenuResolverService to always use the cache when such a request is present. However, if this was done to ensure it is re-fetched in certain situations, we should refactor it to mark the data as stale instead.

Guidance for how to test or review this PR:

• Verify that switching between tabs does not accidentally cause an infinite load. The speed of switching between tabs should have no impact, as this bug is caused by how quickly a response is processed.
• Verify that the DSO edit menus still work correctly and that tabs that don't use them no longer resolve them.

Checklist

• My PR is created against the main branch of code (unless it is a backport or is fixing an issue specific to an older branch).
• 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 npm run lint
• My PR doesn't introduce circular dependencies (verified via npm run 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.
• My PR aligns with Accessibility guidelines if it makes changes to the user interface.
• My PR uses i18n (internationalization) keys instead of hardcoded English text, to allow for translations.
• My PR includes details on how to test it. I've provided clear instructions to reviewers on how to successfully test this fix or feature.
• 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.

[alanorth]

This is missing a port to dspace-7_x tag. Is that because is is related to Angular 16?

[alexandrevryghem]

@alanorth: The BaseDataService fix can be backported to dspace-7_x without any negative side effects. I originally didn't include it because the issue doesn't occur there, as the change that causes the break isn't present yet. However, the menu resolver route optimization could be backported, and we could also include the BaseDataService fix. This way, they would all work consistently.

[kshepherd]

The changes look great. Thanks @alexandrevryghem !
I have had a practical test in my local environment and cannot reproduce the reported problem.

I was hoping the base data service changes might also fix the same "infinite load state" issues in the My DSpace page as well, but no such luck... are there some component-level improvements that we can reuse or apply to My DSpace?

[alexandrevryghem]

@kshepherd: Thnx for the review! I haven't noticed any loading issues on the MyDSpace page, is this also reproducible on the demo servers? I did see however that when you switch between the workflow & workspace tabs that some facet requests are failing but this doesn't seem to cause any infinite loading issues. I also took a quick looked at the components and saw that one observable call could be removed (8849f14), I'll contribute this later but I would need more details on where/when it happens in order to debug this

[pnbecker]

@alexandrevryghem we got some reports from user that my dspace has infinite load times after publishing new items. I was just able to reproduce it on demo by importing and submitting multiple entries and then playing around with the search filters.

[alexandrevryghem]

@pnbecker & @kshepherd: If it’s also occurring on the demo, I would suggest moving this to a separate issue. Could one of you please open a new ticket with steps to reproduce it, and perhaps include a small video?

[pnbecker]

I have no clear way to reproduce it yet. I opened ticket #3697 anyway to gather information if other users run into this too.

[tdonohue]

👍 Thanks @alexandrevryghem ! I gave this a test today and a code review. Code looks good, and I cannot reproduce the bug locally with this PR in place. Merging as this is at +2

[dspace-bot]

Successfully created backport PR for dspace-8_x:

• [Port dspace-8_x] Fix infinite loading on item pages and optimize menu resolver usage #3715

[tdonohue]

@alexandrevryghem : This was ported to 8.x automatically. If there are parts that you feel would be worth porting to 7.x (as you previously mentioned), then I'd gladly review/test them quickly.

As a sidenote, the dspace-7_x branch still has semi-random e2e failures in item-edit.cy.ts, seemingly caused by the tabs not always loading quickly. Therefore, it might be possible that backporting the improvements from this PR would help solve (or improve) that behavior.