Fix infinite loading on item pages and optimize menu resolver usage #3677
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
References
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 theuseCachedVersionIfAvailable
tofalse
. The problem arose due to a change in the timing of when a request is actually sent to the backend. Since Angular 16, anasapScheduler
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 aResponsePending
, which in turn was followed by the new response.Because we don't want to return the old cached value when
useCachedVersionIfAvailable
isfalse
, askipWhile
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 theskipWhile
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 theRemoteData
'sstate
to skip responses. Instead, it should compare thelastUpdated
timestamp with the time before the request was sent. This ensures that all responses emitted by theRemoteDataBuildService#buildList
are skipped unless they are new.Fixed the
no elements in sequence
issue for theItemBitstreamsComponent
andItemRelationshipsComponent
. This was resolved by replacing thefirst()
operators withtake(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:
Checklist
main
branch of code (unless it is a backport or is fixing an issue specific to an older branch).npm run lint
npm run check-circ-deps
)package.json
), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.