Skip to content

Commit 31feb58

Browse files
committed
Fixed still loading of the Saving bar for the CC licenses
1 parent c254cc3 commit 31feb58

3 files changed

Lines changed: 242 additions & 23 deletions

File tree

CC_LICENSE_TESTING_GUIDE.md

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
# Creative Commons License Performance Issue - Testing Guide
2+
3+
## Issue Summary
4+
During submission creation, when selecting Creative Commons license and clicking through its options, the "Saving..." indicator would appear infinitely. After reloading the page, the Creative Commons checkbox would be checked but the license value was not stored in metadata.
5+
6+
## Fix Applied
7+
The issue was resolved by implementing:
8+
1. **Debounced API calls** (300ms delay) to prevent excessive license link requests
9+
2. **Observable caching** using `shareReplay(1)` to avoid redundant HTTP calls
10+
3. **Improved state change detection** to prevent unnecessary JSON patch operations
11+
4. **Proper subscription management** to prevent memory leaks
12+
13+
## Testing Steps
14+
15+
### Before Testing
16+
1. Build and start the DSpace Angular application:
17+
```bash
18+
cd c:\dspace-angular-clarin
19+
npm start
20+
```
21+
22+
### Test Scenario 1: Rapid Option Selection
23+
1. Navigate to submission creation page
24+
2. Choose Creative Commons from the license dropdown
25+
3. **Rapidly click** through different license options (e.g., BY, BY-SA, BY-NC, etc.)
26+
4. **Expected Result**:
27+
- No infinite "Saving..." indicator
28+
- Options respond immediately to clicks
29+
- License link appears/updates smoothly
30+
31+
### Test Scenario 2: License Acceptance and Persistence
32+
1. Select a Creative Commons license type
33+
2. Choose your preferred options (commercial use, derivatives, etc.)
34+
3. **Check the acceptance checkbox** for the generated license link
35+
4. Save the submission (Save for Later)
36+
5. **Reload the page**
37+
6. **Expected Result**:
38+
- Creative Commons section shows as selected
39+
- License URI is properly stored in metadata
40+
- Previous options are preserved
41+
42+
### Test Scenario 3: Performance Verification
43+
1. Open browser developer tools (F12)
44+
2. Go to Network tab
45+
3. Navigate to Creative Commons section
46+
4. Rapidly change license options
47+
5. **Expected Result**:
48+
- Fewer HTTP requests to license URL endpoints
49+
- No duplicate or overlapping requests
50+
- Requests are debounced (not immediate)
51+
52+
### Test Scenario 4: Submission Completion
53+
1. Complete the Creative Commons license selection
54+
2. Fill out other required submission sections
55+
3. Submit/deposit the item
56+
4. **Expected Result**:
57+
- Submission completes successfully
58+
- Creative Commons license metadata is included in final item
59+
60+
## Expected Behavior Changes
61+
62+
### Before Fix
63+
- ❌ Infinite "Saving..." indicator
64+
- ❌ Multiple simultaneous API calls
65+
- ❌ License URI not stored in metadata
66+
- ❌ Poor performance with rapid clicks
67+
68+
### After Fix
69+
- ✅ Smooth interaction with no infinite saving
70+
- ✅ Debounced API calls (max 1 per 300ms)
71+
- ✅ License URI properly persisted
72+
- ✅ Responsive performance even with rapid clicks
73+
74+
## Verification Points
75+
76+
1. **No Console Errors**: Check browser console for any JavaScript errors
77+
2. **Network Efficiency**: Reduced number of HTTP requests in Network tab
78+
3. **Data Persistence**: License data survives page reloads
79+
4. **User Experience**: Smooth, responsive interface interactions
80+
81+
## Rollback Plan
82+
If any issues are discovered, the changes can be easily reverted by restoring the original `submission-section-cc-licenses.component.ts` file from git history.
83+
84+
## Technical Details
85+
The fix is implemented entirely within the Angular component layer and does not affect:
86+
- Backend APIs
87+
- Database schemas
88+
- Other submission sections
89+
- Existing license data
90+
91+
This ensures minimal risk and easy maintenance.

CREATIVE_COMMONS_FIX_CHANGES.md

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
# Creative Commons License Performance Fix
2+
3+
## Problem Description
4+
The Creative Commons license section in the submission form was experiencing infinite saving loops when users clicked on license options. The "Saving..." indicator would appear indefinitely, preventing users from continuing with their submission.
5+
6+
## Root Cause Analysis
7+
The issue was caused by:
8+
9+
1. **Lack of debouncing**: Rapid clicks on CC license options triggered multiple immediate HTTP requests to fetch license links
10+
2. **Inefficient observable handling**: The `ccLicenseLink$` observable was being recreated on every change without proper caching
11+
3. **Overly sensitive state subscriptions**: The section state subscription was triggering unnecessary JSON patch operations on every minor change
12+
13+
## Solution Implemented
14+
15+
### 1. Added Debouncing Mechanism
16+
- Introduced `ccLicenseLinkTrigger$` Subject to control when license link updates are triggered
17+
- Added 300ms debouncing to prevent rapid successive API calls
18+
- Used `switchMap` to cancel previous requests when new ones are triggered
19+
20+
### 2. Improved Observable Caching
21+
- Used `shareReplay(1)` to cache the latest license link result
22+
- Added `distinctUntilChanged` to prevent duplicate emissions
23+
- Proper initialization with `startWith(undefined)`
24+
25+
### 3. Enhanced State Change Detection
26+
- Improved the `distinctUntilChanged` operator to properly compare Creative Commons license data
27+
- Added more precise filtering to prevent unnecessary patch operations
28+
- Only trigger URI operations when acceptance state actually changes
29+
30+
## Code Changes
31+
32+
### Modified Files
33+
- `src/app/submission/sections/cc-license/submission-section-cc-licenses.component.ts`
34+
35+
### Key Changes Made
36+
37+
1. **Added new imports**:
38+
```typescript
39+
import { Subject } from 'rxjs';
40+
import { debounceTime, switchMap, startWith, shareReplay } from 'rxjs/operators';
41+
```
42+
43+
2. **Added debouncing subject**:
44+
```typescript
45+
private ccLicenseLinkTrigger$ = new Subject<void>();
46+
```
47+
48+
3. **Restructured ngOnInit**:
49+
```typescript
50+
this.ccLicenseLink$ = this.ccLicenseLinkTrigger$.pipe(
51+
startWith(undefined),
52+
debounceTime(300),
53+
switchMap(() => this.getCcLicenseLink$() || observableOf(null)),
54+
shareReplay(1),
55+
distinctUntilChanged()
56+
);
57+
```
58+
59+
4. **Updated selection methods**:
60+
- `selectCcLicense()` now triggers `this.ccLicenseLinkTrigger$.next()`
61+
- `selectOption()` now triggers `this.ccLicenseLinkTrigger$.next()`
62+
- `ngOnChanges()` now triggers `this.ccLicenseLinkTrigger$.next()`
63+
64+
5. **Enhanced state subscription**:
65+
- More precise comparison in `distinctUntilChanged`
66+
- Better filtering to prevent unnecessary operations
67+
- Only process URI changes when acceptance state actually changes
68+
69+
6. **Proper cleanup**:
70+
```typescript
71+
onSectionDestroy(): void {
72+
this.subscriptions.forEach((subscription) => subscription.unsubscribe());
73+
this.ccLicenseLinkTrigger$.complete();
74+
}
75+
```
76+
77+
## Expected Results
78+
79+
1. **Performance Improvement**: No more infinite saving loops when clicking CC license options
80+
2. **Better User Experience**: Faster response times due to reduced API calls
81+
3. **Resource Efficiency**: Debounced requests reduce server load
82+
4. **Data Integrity**: License selections are properly saved to metadata after accepting
83+
84+
## Testing Instructions
85+
86+
1. Navigate to submission creation page
87+
2. Select Creative Commons in the license dropdown
88+
3. Rapidly click through different license options
89+
4. Verify that:
90+
- No infinite "Saving..." indicator appears
91+
- License options are properly selected
92+
- After accepting, the license URI is saved to metadata
93+
- Page reload preserves the Creative Commons checkbox state
94+
95+
## Backward Compatibility
96+
This fix maintains complete backward compatibility. No API changes were made, only internal observable handling improvements.

src/app/submission/sections/cc-license/submission-section-cc-licenses.component.ts

Lines changed: 55 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import { ChangeDetectorRef, Component, Inject, OnChanges, SimpleChanges, OnInit } from '@angular/core';
2-
import { Observable, of as observableOf, Subscription, tap } from 'rxjs';
2+
import { Observable, of as observableOf, Subscription, tap, Subject } from 'rxjs';
33
import { Field, Option, SubmissionCcLicence } from '../../../core/submission/models/submission-cc-license.model';
44
import {
55
getFirstCompletedRemoteData, getFirstSucceededRemoteDataPayload,
66
getRemoteDataPayload
77
} from '../../../core/shared/operators';
8-
import { distinctUntilChanged, filter, map, take } from 'rxjs/operators';
8+
import { distinctUntilChanged, filter, map, take, debounceTime, switchMap, startWith, shareReplay } from 'rxjs/operators';
99
import { SubmissionCcLicenseDataService } from '../../../core/submission/submission-cc-license-data.service';
1010
import { NgbModal, NgbModalRef } from '@ng-bootstrap/ng-bootstrap';
1111
import { renderSectionFor } from '../sections-decorator';
@@ -89,6 +89,11 @@ export class SubmissionSectionCcLicensesComponent extends SectionModelComponent
8989
*/
9090
private _isLastPage: boolean;
9191

92+
/**
93+
* Subject to trigger CC license link updates with debouncing
94+
*/
95+
private ccLicenseLinkTrigger$ = new Subject<void>();
96+
9297
/**
9398
* The Creative Commons link saved in the workspace item.
9499
*/
@@ -129,14 +134,20 @@ export class SubmissionSectionCcLicensesComponent extends SectionModelComponent
129134

130135
ngOnInit(): void {
131136
super.ngOnInit();
132-
if (hasNoValue(this.ccLicenseLink$)) {
133-
this.ccLicenseLink$ = this.getCcLicenseLink$();
134-
}
137+
// Initialize the debounced license link observable
138+
this.ccLicenseLink$ = this.ccLicenseLinkTrigger$.pipe(
139+
startWith(undefined), // Start with initial trigger
140+
debounceTime(300), // Debounce rapid clicks
141+
switchMap(() => this.getCcLicenseLink$() || observableOf(null)),
142+
shareReplay(1), // Cache the latest result
143+
distinctUntilChanged()
144+
);
135145
}
136146

137147
ngOnChanges(changes: SimpleChanges): void {
138148
if (hasValue(changes.sectionData) || hasValue(changes.submissionCcLicenses)) {
139-
this.ccLicenseLink$ = this.getCcLicenseLink$();
149+
// Trigger the debounced license link update
150+
this.ccLicenseLinkTrigger$.next();
140151
}
141152
}
142153

@@ -164,7 +175,8 @@ export class SubmissionSectionCcLicensesComponent extends SectionModelComponent
164175
},
165176
uri: undefined,
166177
});
167-
this.ccLicenseLink$ = this.getCcLicenseLink$();
178+
// Trigger the debounced license link update
179+
this.ccLicenseLinkTrigger$.next();
168180
}
169181

170182
/**
@@ -196,7 +208,8 @@ export class SubmissionSectionCcLicensesComponent extends SectionModelComponent
196208
},
197209
accepted: false,
198210
});
199-
this.ccLicenseLink$ = this.getCcLicenseLink$();
211+
// Trigger the debounced license link update
212+
this.ccLicenseLinkTrigger$.next();
200213
}
201214

202215
/**
@@ -272,6 +285,8 @@ export class SubmissionSectionCcLicensesComponent extends SectionModelComponent
272285
*/
273286
onSectionDestroy(): void {
274287
this.subscriptions.forEach((subscription) => subscription.unsubscribe());
288+
// Complete the subject to prevent memory leaks
289+
this.ccLicenseLinkTrigger$.complete();
275290
}
276291

277292
/**
@@ -284,18 +299,35 @@ export class SubmissionSectionCcLicensesComponent extends SectionModelComponent
284299
filter((sectionState) => {
285300
return isNotEmpty(sectionState) && (isNotEmpty(sectionState.data) || isNotEmpty(sectionState.errorsToShow));
286301
}),
287-
distinctUntilChanged(),
302+
distinctUntilChanged((prev, curr) => {
303+
// More precise comparison to prevent unnecessary updates
304+
const prevData = prev?.data as WorkspaceitemSectionCcLicenseObject;
305+
const currData = curr?.data as WorkspaceitemSectionCcLicenseObject;
306+
return prevData?.accepted === currData?.accepted &&
307+
prevData?.uri === currData?.uri &&
308+
JSON.stringify(prevData?.ccLicense) === JSON.stringify(currData?.ccLicense);
309+
}),
288310
map((sectionState) => sectionState.data as WorkspaceitemSectionCcLicenseObject),
289311
).subscribe((data) => {
290-
if (this.data.accepted !== data.accepted) {
312+
const wasAccepted = this.data.accepted;
313+
const wasUri = this.data.uri;
314+
315+
// Only process if acceptance state actually changed
316+
if (wasAccepted !== data.accepted && data.accepted !== undefined) {
291317
const path = this.pathCombiner.getPath('uri');
292-
if (data.accepted) {
293-
this.getCcLicenseLink$().pipe(
294-
take(1),
295-
).subscribe((link) => {
296-
this.operationsBuilder.add(path, link.toString(), false, true);
297-
});
298-
} else if (!!this.data.uri) {
318+
if (data.accepted && !wasAccepted) {
319+
// Only add URI if we're switching from not accepted to accepted
320+
const licenseLink$ = this.getCcLicenseLink$();
321+
if (licenseLink$) {
322+
licenseLink$.pipe(
323+
take(1),
324+
filter(link => !!link && link !== wasUri) // Only proceed if link exists and is different
325+
).subscribe((link) => {
326+
this.operationsBuilder.add(path, link.toString(), false, true);
327+
});
328+
}
329+
} else if (!data.accepted && wasAccepted && !!this.data.uri) {
330+
// Only remove URI if we're switching from accepted to not accepted
299331
this.operationsBuilder.remove(path);
300332
}
301333
}
@@ -305,12 +337,12 @@ export class SubmissionSectionCcLicensesComponent extends SectionModelComponent
305337
getFirstCompletedRemoteData(),
306338
getRemoteDataPayload()
307339
).subscribe((remoteData) => {
308-
if (remoteData === undefined || remoteData.values.length === 0) {
309-
// No value configured, use blank value (International jurisdiction)
310-
this.defaultJurisdiction = '';
311-
} else {
312-
this.defaultJurisdiction = remoteData.values[0];
313-
}
340+
if (remoteData === undefined || remoteData.values.length === 0) {
341+
// No value configured, use blank value (International jurisdiction)
342+
this.defaultJurisdiction = '';
343+
} else {
344+
this.defaultJurisdiction = remoteData.values[0];
345+
}
314346
})
315347
);
316348
this.loadCcLicences();

0 commit comments

Comments
 (0)