Patch operation returns SuccessStale instead of Success

[davide-negretti]

Describe the bug

The state of remote data returned by patch() method of REST data services is SuccessStale when Success is expected.

This happens e.g. in submit() method of dso-edit-metadata.component.ts. When I add or change a metadata the subscription returns a RD object with the updated item as payload, but SucessStale as state:

dspace-angular/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata.component.ts

Lines 182 to 186 in a57bcf0

	submit(): void {
	this.saving$.next(true);
	this.updateDataService.patch(this.dso, this.form.getOperations(this.arrayMoveChangeAnalyser)).pipe(
	getFirstCompletedRemoteData()
	).subscribe((rd: RemoteData<DSpaceObject>) => {

To Reproduce

Either add a breakpoint at this.saving$.next(false); inside the subscription to submit() method or log the value of rd, then:

1. Login as administrator
2. Open any item
3. Choose "Administer" from context menu
4. Open "Metadata" tab
5. Edit any existing metadata or add a new one
6. Save
   The payload of the response contains the updated metadata, but rd.state is SuccessStale

Expected behavior
rd.state should be Success

[tdonohue]

I'm having difficulty understanding the description here. Could you link to the code where the issue exists?

This sounds loosely similar to #2349 , at least in that it impacts a similar area of code. But, I'm not sure it's the same problem... it just sounds similar to me. So it's possible it was a similar bug accidentally caused by cache redesign work in #961

Overall, it sounds like a bug. I just don't understand enough about how/where this exists.

@artlowel : Pinging you in case your team is interested in this, just because it sounds like it might be similar to past work your team has done.

[hutattedonmyarm]

Seems to be related to DSpace/DSpace#8925, specifically the first point. To quote myself:

We noticed the same issue. When checking the response in orcid-sync-settings.component.ts, it is done using remoteData.isSuccess, but the response is usually a stale success, so it falls through:

[davide-negretti]

@tdonohue sorry, some lines were missing from the bug description, I fixed it and added the code reference.

In my understanding, if the payload contains the updated data then the state should be Success.

@hutattedonmyarm yes, the issues are related, but we need to understand, case by case, whether:

• it is correct to have the SuccessStale state (and we would need to use hasSucceeded() instead of isSuccess() in order to consider a stale operation as successful)
• the state should be Success and it is correct to consider a SuccessStale response as non successful

[hutattedonmyarm]

@davide-negretti absolutely! I mentioned the issue, because the ORCID settings page does use a PATCH request and will always return SuccessStale regardless if it is correct or not. Our workaround to prevent errornous error messages is just that: A workaround. I didn't look further into the issue if it might be a general issue with PATCH requests