From b460a2440cdbfc25b01aa7d8782b653a5761ea9d Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Thu, 19 Mar 2026 13:45:52 +0100 Subject: [PATCH 1/5] removed subcription in favour of input --- .../data-visibility-control.component.ts | 27 +++++++---------- .../general-access-control.component.ts | 30 +++++++------------ .../share-dialog/share-dialog.component.ts | 23 ++++---------- .../shared/share-list/share-list.component.ts | 25 ++++++---------- .../share-survey/share-survey.component.html | 6 ++-- 5 files changed, 38 insertions(+), 73 deletions(-) diff --git a/web/src/app/components/shared/data-visibility-control/data-visibility-control.component.ts b/web/src/app/components/shared/data-visibility-control/data-visibility-control.component.ts index 3c24c1717..c48425f3d 100644 --- a/web/src/app/components/shared/data-visibility-control/data-visibility-control.component.ts +++ b/web/src/app/components/shared/data-visibility-control/data-visibility-control.component.ts @@ -14,9 +14,8 @@ * limitations under the License. */ -import { Component } from '@angular/core'; +import { Component, effect, input } from '@angular/core'; import { MatSlideToggleChange } from '@angular/material/slide-toggle'; -import { Subscription } from 'rxjs'; import { Survey, SurveyDataVisibility } from 'app/models/survey.model'; import { AuthService } from 'app/services/auth/auth.service'; @@ -28,7 +27,7 @@ import { DraftSurveyService } from 'app/services/draft-survey/draft-survey.servi standalone: false, }) export class DataVisibilityControlComponent { - private subscription = new Subscription(); + survey = input(); selectedDataVisibility!: SurveyDataVisibility; @@ -38,16 +37,14 @@ export class DataVisibilityControlComponent { readonly authService: AuthService, readonly draftSurveyService: DraftSurveyService ) { - this.subscription.add( - this.draftSurveyService - .getSurvey$() - .subscribe(survey => this.onSurveyLoaded(survey)) - ); - } - - private async onSurveyLoaded(survey: Survey): Promise { - this.selectedDataVisibility = - survey.dataVisibility || SurveyDataVisibility.CONTRIBUTOR_AND_ORGANIZERS; + effect(() => { + const survey = this.survey(); + if (survey) { + this.selectedDataVisibility = + survey.dataVisibility || + SurveyDataVisibility.CONTRIBUTOR_AND_ORGANIZERS; + } + }); } changeDataVisibility(event: MatSlideToggleChange) { @@ -59,8 +56,4 @@ export class DataVisibilityControlComponent { this.draftSurveyService.updateDataVisibility(dataVisibility); } - - ngOnDestroy() { - this.subscription.unsubscribe(); - } } diff --git a/web/src/app/components/shared/general-access-control/general-access-control.component.ts b/web/src/app/components/shared/general-access-control/general-access-control.component.ts index 56d022797..ad4eced74 100644 --- a/web/src/app/components/shared/general-access-control/general-access-control.component.ts +++ b/web/src/app/components/shared/general-access-control/general-access-control.component.ts @@ -16,9 +16,8 @@ import '@angular/localize/init'; -import { Component } from '@angular/core'; +import { Component, effect, input } from '@angular/core'; import { Map } from 'immutable'; -import { Subscription } from 'rxjs'; import { Survey, SurveyGeneralAccess } from 'app/models/survey.model'; import { AuthService } from 'app/services/auth/auth.service'; @@ -53,7 +52,7 @@ const generalAccessLabels = Map< standalone: false, }) export class GeneralAccessControlComponent { - private subscription = new Subscription(); + survey = input(); selectedGeneralAccess!: SurveyGeneralAccess; @@ -65,31 +64,24 @@ export class GeneralAccessControlComponent { readonly authService: AuthService, readonly draftSurveyService: DraftSurveyService ) { - this.subscription.add( - this.draftSurveyService - .getSurvey$() - .subscribe(survey => this.onSurveyLoaded(survey)) - ); + effect(() => { + const survey = this.survey(); + if (survey) { + // Default to RESTRICTED for general access if not explicitly set. + // This is present for backward-compatibility with older surveys. + this.selectedGeneralAccess = + survey.generalAccess || SurveyGeneralAccess.RESTRICTED; + } + }); } get generalAccessKeys(): SurveyGeneralAccess[] { return Array.from(this.generalAccessLabels.keys()); } - private async onSurveyLoaded(survey: Survey): Promise { - // Default to RESTRICTED for general access if not explicitly set. - // This is present for backward-compatibility with older surveys. - this.selectedGeneralAccess = - survey.generalAccess || SurveyGeneralAccess.RESTRICTED; - } - changeGeneralAccess(generalAccess: SurveyGeneralAccess) { this.selectedGeneralAccess = generalAccess; this.draftSurveyService.updateGeneralAccess(generalAccess); } - - ngOnDestroy() { - this.subscription.unsubscribe(); - } } diff --git a/web/src/app/components/shared/share-dialog/share-dialog.component.ts b/web/src/app/components/shared/share-dialog/share-dialog.component.ts index baefba2b0..e14c1a6d9 100644 --- a/web/src/app/components/shared/share-dialog/share-dialog.component.ts +++ b/web/src/app/components/shared/share-dialog/share-dialog.component.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { Component } from '@angular/core'; +import { Component, Inject } from '@angular/core'; import { AbstractControl, FormControl, @@ -23,10 +23,9 @@ import { ValidatorFn, Validators, } from '@angular/forms'; -import { MatDialogRef } from '@angular/material/dialog'; +import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog'; import { MatSelectChange } from '@angular/material/select'; import { Map } from 'immutable'; -import { Subscription } from 'rxjs'; import { AclEntry } from 'app/models/acl-entry.model'; import { Role } from 'app/models/role.model'; @@ -65,17 +64,12 @@ export class ShareDialogComponent { /** The active survey. */ private survey?: Survey; - private subscription = new Subscription(); - constructor( private dialogRef: MatDialogRef, - private draftSurveyService: DraftSurveyService + private draftSurveyService: DraftSurveyService, + @Inject(MAT_DIALOG_DATA) data: { survey: Survey } ) { - this.subscription.add( - this.draftSurveyService - .getSurvey$() - .subscribe(survey => this.onSurveyLoaded(survey)) - ); + this.onSurveyLoaded(data.survey); } /** @@ -128,13 +122,6 @@ export class ShareDialogComponent { this.dialogRef.close(); } - /** - * Clean up Rx subscription when cleaning up the component. - */ - ngOnDestroy(): void { - this.subscription.unsubscribe(); - } - /** * Update ACL and surveyId when survey is loaded. */ diff --git a/web/src/app/components/shared/share-list/share-list.component.ts b/web/src/app/components/shared/share-list/share-list.component.ts index 0b1326f26..7e3851cef 100644 --- a/web/src/app/components/shared/share-list/share-list.component.ts +++ b/web/src/app/components/shared/share-list/share-list.component.ts @@ -14,10 +14,9 @@ * limitations under the License. */ -import { Component } from '@angular/core'; +import { Component, effect, input } from '@angular/core'; import { MatSelectChange } from '@angular/material/select'; import { Map } from 'immutable'; -import { Subscription } from 'rxjs'; import { AclEntry } from 'app/models/acl-entry.model'; import { Role } from 'app/models/role.model'; @@ -32,12 +31,11 @@ import { DraftSurveyService } from 'app/services/draft-survey/draft-survey.servi standalone: false, }) export class ShareListComponent { + survey = input(); + acl: Array = []; - survey?: Survey; surveyOwnerEmail = ''; - private subscription = new Subscription(); - readonly roleOptions = ROLE_OPTIONS; roles = Role; @@ -46,16 +44,15 @@ export class ShareListComponent { readonly authService: AuthService, readonly draftSurveyService: DraftSurveyService ) { - this.subscription.add( - this.draftSurveyService - .getSurvey$() - .subscribe(survey => this.onSurveyLoaded(survey)) - ); + effect(() => { + const survey = this.survey(); + if (survey) { + this.onSurveyLoaded(survey); + } + }); } private async onSurveyLoaded(survey: Survey): Promise { - this.survey = survey; - const owner = await this.authService.getUser(survey.ownerId); this.surveyOwnerEmail = owner?.email || ''; @@ -88,8 +85,4 @@ export class ShareListComponent { Map(aclUpdate.map(entry => [entry.email, entry.role])) ); } - - ngOnDestroy() { - this.subscription.unsubscribe(); - } } diff --git a/web/src/app/components/shared/share-survey/share-survey.component.html b/web/src/app/components/shared/share-survey/share-survey.component.html index f7892ab88..506d53034 100644 --- a/web/src/app/components/shared/share-survey/share-survey.component.html +++ b/web/src/app/components/shared/share-survey/share-survey.component.html @@ -27,7 +27,7 @@ - + @@ -37,12 +37,12 @@ - + - + Data visibility From 23d4689cfed77a56b641fcd897eaa1eeed3158ed Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Thu, 19 Mar 2026 14:15:30 +0100 Subject: [PATCH 2/5] added survey resolver to pass survey to share survey component --- .../create-survey.component.html | 1 + .../share-dialog.component.spec.ts | 19 +++++++++++--- .../share-list/share-list.component.spec.ts | 11 +++----- .../share-survey/share-survey.component.ts | 6 ++++- web/src/app/resolvers/survey.resolver.ts | 25 +++++++++++++++++++ web/src/app/routing.module.ts | 7 +++++- 6 files changed, 56 insertions(+), 13 deletions(-) create mode 100644 web/src/app/resolvers/survey.resolver.ts diff --git a/web/src/app/components/create-survey/create-survey.component.html b/web/src/app/components/create-survey/create-survey.component.html index 1df704780..4641bd137 100644 --- a/web/src/app/components/create-survey/create-survey.component.html +++ b/web/src/app/components/create-survey/create-survey.component.html @@ -95,6 +95,7 @@ > diff --git a/web/src/app/components/shared/share-dialog/share-dialog.component.spec.ts b/web/src/app/components/shared/share-dialog/share-dialog.component.spec.ts index 937aa2b14..ae96997ea 100644 --- a/web/src/app/components/shared/share-dialog/share-dialog.component.spec.ts +++ b/web/src/app/components/shared/share-dialog/share-dialog.component.spec.ts @@ -17,15 +17,15 @@ import { ComponentFixture, TestBed } from '@angular/core/testing'; import { FormsModule, ReactiveFormsModule } from '@angular/forms'; import { MatButtonModule } from '@angular/material/button'; -import { MatDialogModule, MatDialogRef } from '@angular/material/dialog'; +import { MAT_DIALOG_DATA, MatDialogModule, MatDialogRef } from '@angular/material/dialog'; import { MatFormFieldModule } from '@angular/material/form-field'; import { MatInputModule } from '@angular/material/input'; import { MatListModule } from '@angular/material/list'; import { MatSelectModule } from '@angular/material/select'; import { NoopAnimationsModule } from '@angular/platform-browser/animations'; import { Map } from 'immutable'; -import { of } from 'rxjs'; +import { DataSharingType, Survey } from 'app/models/survey.model'; import { DraftSurveyService } from 'app/services/draft-survey/draft-survey.service'; import { ShareDialogComponent } from './share-dialog.component'; @@ -52,9 +52,20 @@ describe('ShareDialogComponent', () => { { provide: MatDialogRef, useValue: {} }, { provide: DraftSurveyService, + useValue: { updateAcl: () => null }, + }, + { + provide: MAT_DIALOG_DATA, useValue: { - getSurvey$: () => of({ acl: Map(), getAclEntriesSorted: () => [] }), - updateAcl: () => null, + survey: new Survey( + 'id', + 'title', + 'description', + Map(), + Map(), + 'owner@example.com', + { type: DataSharingType.PRIVATE } + ), }, }, ], diff --git a/web/src/app/components/shared/share-list/share-list.component.spec.ts b/web/src/app/components/shared/share-list/share-list.component.spec.ts index 2dfdfd12c..e2b3199cc 100644 --- a/web/src/app/components/shared/share-list/share-list.component.spec.ts +++ b/web/src/app/components/shared/share-list/share-list.component.spec.ts @@ -18,7 +18,6 @@ import { ComponentFixture, TestBed } from '@angular/core/testing'; import { MatListModule } from '@angular/material/list'; import { MatSelectChange, MatSelectModule } from '@angular/material/select'; import { Map } from 'immutable'; -import { Subject } from 'rxjs'; import { Role } from 'app/models/role.model'; import { DataSharingType, Survey } from 'app/models/survey.model'; @@ -34,23 +33,19 @@ describe('ShareListComponent', () => { let draftSurveyServiceSpy: jasmine.SpyObj; let authServiceSpy: jasmine.SpyObj; - let activeSurvey$: Subject; const user = new User('user1', 'user1@gmail.com', true); beforeEach(async () => { draftSurveyServiceSpy = jasmine.createSpyObj( 'DraftSurveyService', - ['getSurvey$', 'updateAcl'] + ['updateAcl'] ); authServiceSpy = jasmine.createSpyObj('AuthService', [ 'getUser', ]); - activeSurvey$ = new Subject(); - - draftSurveyServiceSpy.getSurvey$.and.returnValue(activeSurvey$); authServiceSpy.getUser.and.callFake(email => { if (email === 'owner-email') { return Promise.resolve(new User('owner', 'owner@gmail.com', true)); @@ -79,7 +74,8 @@ describe('ShareListComponent', () => { }); it('updates itself when acl changes', async () => { - activeSurvey$.next( + fixture.componentRef.setInput( + 'survey', new Survey( 'id', 'title', @@ -90,6 +86,7 @@ describe('ShareListComponent', () => { { type: DataSharingType.PRIVATE } ) ); + fixture.detectChanges(); await fixture.whenStable(); fixture.detectChanges(); diff --git a/web/src/app/components/shared/share-survey/share-survey.component.ts b/web/src/app/components/shared/share-survey/share-survey.component.ts index ae7051a33..9b2da37b2 100644 --- a/web/src/app/components/shared/share-survey/share-survey.component.ts +++ b/web/src/app/components/shared/share-survey/share-survey.component.ts @@ -1,7 +1,8 @@ -import { Component } from '@angular/core'; +import { Component, input } from '@angular/core'; import { MatDialog } from '@angular/material/dialog'; import { ShareDialogComponent } from 'app/components/shared/share-dialog/share-dialog.component'; +import { Survey } from 'app/models/survey.model'; @Component({ selector: 'share-survey', @@ -10,12 +11,15 @@ import { ShareDialogComponent } from 'app/components/shared/share-dialog/share-d standalone: false, }) export class ShareSurveyComponent { + survey = input(); + constructor(private dialog: MatDialog) {} openShareDialog(): void { this.dialog.open(ShareDialogComponent, { width: '580px', autoFocus: false, + data: { survey: this.survey() }, }); } } diff --git a/web/src/app/resolvers/survey.resolver.ts b/web/src/app/resolvers/survey.resolver.ts new file mode 100644 index 000000000..76cf09032 --- /dev/null +++ b/web/src/app/resolvers/survey.resolver.ts @@ -0,0 +1,25 @@ +/** + * Copyright 2026 The Ground Authors. + * + * Licensed under the Apache License, Version 2.0 (the 'License'); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an 'AS IS' BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { inject } from '@angular/core'; +import { ResolveFn } from '@angular/router'; +import { take } from 'rxjs'; + +import { Survey } from 'app/models/survey.model'; +import { DraftSurveyService } from 'app/services/draft-survey/draft-survey.service'; + +export const surveyResolver: ResolveFn = () => + inject(DraftSurveyService).getSurvey$().pipe(take(1)); diff --git a/web/src/app/routing.module.ts b/web/src/app/routing.module.ts index 242bc6229..74b9b046d 100644 --- a/web/src/app/routing.module.ts +++ b/web/src/app/routing.module.ts @@ -61,6 +61,7 @@ import { SurveyJsonModule } from './components/edit-survey/survey-json/survey-js import { ErrorComponent } from './components/error/error.component'; import { ErrorModule } from './components/error/error.module'; import { ShareSurveyComponent } from './components/shared/share-survey/share-survey.component'; +import { surveyResolver } from './resolvers/survey.resolver'; import { TermsComponent } from './components/terms/terms.component'; import { TermsModule } from './components/terms/terms.module'; import { dirtyCheckGuard } from './guards/dirty-check.guard'; @@ -99,7 +100,11 @@ const routes: Routes = [ children: [ { path: 'job/:id', component: EditJobComponent }, { path: 'survey', component: EditDetailsComponent }, - { path: 'share', component: ShareSurveyComponent }, + { + path: 'share', + component: ShareSurveyComponent, + resolve: { survey: surveyResolver }, + }, { path: 'json', component: SurveyJsonComponent }, ], }, From 7752b143835bab6624dbffaff0011ddd22ff906f Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Thu, 19 Mar 2026 14:19:51 +0100 Subject: [PATCH 3/5] fixed signal calls --- .../shared/share-survey/share-survey.component.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/web/src/app/components/shared/share-survey/share-survey.component.html b/web/src/app/components/shared/share-survey/share-survey.component.html index 506d53034..dda7860cf 100644 --- a/web/src/app/components/shared/share-survey/share-survey.component.html +++ b/web/src/app/components/shared/share-survey/share-survey.component.html @@ -27,7 +27,7 @@ - + @@ -37,12 +37,12 @@ - + - + Data visibility From 430ea7e39f87cc01ed479b96b15335802efdbe0e Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Thu, 19 Mar 2026 14:22:25 +0100 Subject: [PATCH 4/5] reverted surveyresolver --- .../create-survey.component.html | 1 - .../share-survey/share-survey.component.ts | 10 +++++--- web/src/app/resolvers/survey.resolver.ts | 25 ------------------- web/src/app/routing.module.ts | 7 +----- 4 files changed, 7 insertions(+), 36 deletions(-) delete mode 100644 web/src/app/resolvers/survey.resolver.ts diff --git a/web/src/app/components/create-survey/create-survey.component.html b/web/src/app/components/create-survey/create-survey.component.html index 4641bd137..1df704780 100644 --- a/web/src/app/components/create-survey/create-survey.component.html +++ b/web/src/app/components/create-survey/create-survey.component.html @@ -95,7 +95,6 @@ > diff --git a/web/src/app/components/shared/share-survey/share-survey.component.ts b/web/src/app/components/shared/share-survey/share-survey.component.ts index 9b2da37b2..9c1d1bedf 100644 --- a/web/src/app/components/shared/share-survey/share-survey.component.ts +++ b/web/src/app/components/shared/share-survey/share-survey.component.ts @@ -1,8 +1,9 @@ -import { Component, input } from '@angular/core'; +import { Component, inject } from '@angular/core'; +import { toSignal } from '@angular/core/rxjs-interop'; import { MatDialog } from '@angular/material/dialog'; import { ShareDialogComponent } from 'app/components/shared/share-dialog/share-dialog.component'; -import { Survey } from 'app/models/survey.model'; +import { DraftSurveyService } from 'app/services/draft-survey/draft-survey.service'; @Component({ selector: 'share-survey', @@ -11,9 +12,10 @@ import { Survey } from 'app/models/survey.model'; standalone: false, }) export class ShareSurveyComponent { - survey = input(); + private draftSurveyService = inject(DraftSurveyService); + private dialog = inject(MatDialog); - constructor(private dialog: MatDialog) {} + survey = toSignal(this.draftSurveyService.getSurvey$()); openShareDialog(): void { this.dialog.open(ShareDialogComponent, { diff --git a/web/src/app/resolvers/survey.resolver.ts b/web/src/app/resolvers/survey.resolver.ts deleted file mode 100644 index 76cf09032..000000000 --- a/web/src/app/resolvers/survey.resolver.ts +++ /dev/null @@ -1,25 +0,0 @@ -/** - * Copyright 2026 The Ground Authors. - * - * Licensed under the Apache License, Version 2.0 (the 'License'); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an 'AS IS' BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { inject } from '@angular/core'; -import { ResolveFn } from '@angular/router'; -import { take } from 'rxjs'; - -import { Survey } from 'app/models/survey.model'; -import { DraftSurveyService } from 'app/services/draft-survey/draft-survey.service'; - -export const surveyResolver: ResolveFn = () => - inject(DraftSurveyService).getSurvey$().pipe(take(1)); diff --git a/web/src/app/routing.module.ts b/web/src/app/routing.module.ts index 74b9b046d..242bc6229 100644 --- a/web/src/app/routing.module.ts +++ b/web/src/app/routing.module.ts @@ -61,7 +61,6 @@ import { SurveyJsonModule } from './components/edit-survey/survey-json/survey-js import { ErrorComponent } from './components/error/error.component'; import { ErrorModule } from './components/error/error.module'; import { ShareSurveyComponent } from './components/shared/share-survey/share-survey.component'; -import { surveyResolver } from './resolvers/survey.resolver'; import { TermsComponent } from './components/terms/terms.component'; import { TermsModule } from './components/terms/terms.module'; import { dirtyCheckGuard } from './guards/dirty-check.guard'; @@ -100,11 +99,7 @@ const routes: Routes = [ children: [ { path: 'job/:id', component: EditJobComponent }, { path: 'survey', component: EditDetailsComponent }, - { - path: 'share', - component: ShareSurveyComponent, - resolve: { survey: surveyResolver }, - }, + { path: 'share', component: ShareSurveyComponent }, { path: 'json', component: SurveyJsonComponent }, ], }, From 23ac02cc060c0c4e3da4ccb9e18b67168a6d38c6 Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Thu, 19 Mar 2026 14:30:32 +0100 Subject: [PATCH 5/5] fixed test --- .../shared/share-survey/share-survey.component.spec.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/web/src/app/components/shared/share-survey/share-survey.component.spec.ts b/web/src/app/components/shared/share-survey/share-survey.component.spec.ts index 17bc61152..ea3d8ddba 100644 --- a/web/src/app/components/shared/share-survey/share-survey.component.spec.ts +++ b/web/src/app/components/shared/share-survey/share-survey.component.spec.ts @@ -3,6 +3,9 @@ import { ComponentFixture, TestBed } from '@angular/core/testing'; import { MatCardModule } from '@angular/material/card'; import { MatDialogModule } from '@angular/material/dialog'; import { MatIconModule } from '@angular/material/icon'; +import { NEVER } from 'rxjs'; + +import { DraftSurveyService } from 'app/services/draft-survey/draft-survey.service'; import { ShareSurveyComponent } from './share-survey.component'; @@ -15,6 +18,12 @@ describe('ShareSurveyComponent', () => { imports: [MatIconModule, MatDialogModule, MatCardModule], declarations: [ShareSurveyComponent], schemas: [NO_ERRORS_SCHEMA], + providers: [ + { + provide: DraftSurveyService, + useValue: { getSurvey$: () => NEVER }, + }, + ], }).compileComponents(); fixture = TestBed.createComponent(ShareSurveyComponent);