Skip to content

Commit 802bcd6

Browse files
committed
users: smoother contact visibility and leader only links
- contact opt in: email and phone on a profile were readable by every logged in user, including a nation browsing a community's leaders. They are now behind per field toggles in Edit Profile, with the member themselves and local planet admins keeping the full view. Absent flags read as opted out - member links are gated to community leaders (canHoldMemberLinks) until the wider product direction is settled; opening it to all members is deleting the calls to that predicate - UserService.updateUser no longer spreads the logged in user's password fields onto another user's doc, which locked that user out whenever the caller passed a credential stripped copy such as the profile view's Notes marked "TEMP NOTE (for review, strip before merge)" flag the temporary gate and the two behaviour changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Duco31c9dD2B9xdCeXbfgj
1 parent c55c819 commit 802bcd6

14 files changed

Lines changed: 243 additions & 29 deletions

src/app/community/community.component.spec.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,10 +170,20 @@ describe('CommunityComponent remote exchange behavior', () => {
170170
expect(component.deleteMode).toBe(false);
171171
});
172172

173+
// TEMP NOTE (for review, strip before merge): links are leaders only for now.
174+
it('does not offer link editing for a member who is not a leader', () => {
175+
const { component } = createComponent();
176+
component.planetCode = null;
177+
component.user = { ...component.user, roles: [ 'manager' ] };
178+
179+
expect(component.councillorActionMenu({ userId: 'org.couchdb.user:bob', userPlanetCode: 'local', doc: { roles: [] } }))
180+
.toEqual([ 'title' ]);
181+
});
182+
173183
it('offers title changes to managers and link editing to managers and the leader themselves', () => {
174184
const { component } = createComponent();
175-
const councillor = { userId: 'user', userPlanetCode: 'local' };
176-
const otherCouncillor = { userId: 'org.couchdb.user:bob', userPlanetCode: 'local' };
185+
const councillor = { userId: 'user', userPlanetCode: 'local', doc: { roles: [ 'leader' ] } };
186+
const otherCouncillor = { userId: 'org.couchdb.user:bob', userPlanetCode: 'local', doc: { roles: [ 'leader' ] } };
177187

178188
expect(component.councillorActionMenu(councillor)).toEqual([]);
179189

src/app/community/community.component.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { PlanetMessageService } from '../shared/planet-message.service';
1616
import { UserService } from '../shared/user.service';
1717
import { UsersService } from '../users/users.service';
1818
import { UsersLinksService } from '../users/users-links.service';
19+
import { canHoldMemberLinks } from '../shared/social-platforms.constants';
1920
import { findDocuments } from '../shared/mangoQueries';
2021
import { CustomValidators } from '../validators/custom-validators';
2122
import { environment } from '../../environments/environment';
@@ -579,9 +580,12 @@ export class CommunityComponent implements OnInit, OnDestroy {
579580
}
580581
const canManageMembers = this.user.roles.indexOf('_admin') > -1 || this.user.roles.indexOf('manager') > -1;
581582
const isSelf = councillor.userId === this.user._id && councillor.userPlanetCode === this.stateService.configuration.code;
583+
// The tab only lists leaders and admins, so canHoldMemberLinks is redundant here today. It
584+
// is kept so the leaders only gate stays in one predicate.
585+
const canEditLinks = canHoldMemberLinks(councillor.doc) && (canManageMembers || isSelf);
582586
return [
583587
...(canManageMembers ? [ 'title' as const ] : []),
584-
...(canManageMembers || isSelf ? [ 'links' as const ] : [])
588+
...(canEditLinks ? [ 'links' as const ] : [])
585589
];
586590
}
587591

src/app/shared/social-platforms.constants.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@ export interface MemberLink {
3636
label?: string;
3737
}
3838

39+
// TEMP NOTE (for review, strip before merge): member links ship for community leaders only
40+
// while the wider product direction is decided. Everything else is already in place for all
41+
// members, so opening it up is deleting the calls to this predicate -- nothing else. Display is
42+
// deliberately not gated: only leaders can create links, so nobody else has any to show.
43+
export const canHoldMemberLinks = (userDoc: any): boolean =>
44+
userDoc?.isUserAdmin === true || (userDoc?.roles || []).indexOf('leader') > -1;
45+
3946
const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
4047
const phoneRegex = /^\+?[\d\s().-]{4,20}$/;
4148

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import { vi } from 'vitest';
2+
import { of } from 'rxjs';
3+
import { CouchService } from './couchdb.service';
4+
import { StateService } from './state.service';
5+
import { UserService } from './user.service';
6+
7+
describe('UserService', () => {
8+
let service: UserService;
9+
let couchService: { put: ReturnType<typeof vi.fn>, get: ReturnType<typeof vi.fn> };
10+
11+
const adminCredentials = { derived_key: 'admin-key', salt: 'admin-salt', iterations: 10 };
12+
const putBody = () => couchService.put.mock.calls[0][1];
13+
14+
beforeEach(() => {
15+
couchService = {
16+
put: vi.fn(() => of({ ok: true })),
17+
get: vi.fn(() => of({ _id: 'org.couchdb.user:admin', name: 'admin', roles: [] }))
18+
};
19+
service = new UserService(
20+
couchService as any as CouchService,
21+
{ configuration: { _id: 'configuration', adminName: 'other@local', code: 'local' }, requestData: vi.fn() } as any as StateService
22+
);
23+
service.set({ _id: 'org.couchdb.user:admin', name: 'admin', roles: [ 'manager' ] });
24+
service.credentials = adminCredentials;
25+
});
26+
27+
afterEach(() => {
28+
vi.restoreAllMocks();
29+
});
30+
31+
describe('updateUser', () => {
32+
// TEMP NOTE (for review, strip before merge): this is the lockout described in
33+
// user.service.ts. The profile view strips credential fields before handing a doc to a
34+
// caller, so an admin editing that copy used to write their own password hash onto it.
35+
it('does not fall back to the editor credentials when saving another user', () => {
36+
service.updateUser({ _id: 'org.couchdb.user:ann', name: 'ann', roles: [] }).subscribe();
37+
38+
expect(putBody().derived_key).toBeUndefined();
39+
expect(putBody().salt).toBeUndefined();
40+
});
41+
42+
it('keeps the credential fallback for the logged in user own doc', () => {
43+
service.updateUser({ _id: 'org.couchdb.user:admin', name: 'admin', roles: [] }).subscribe();
44+
45+
expect(putBody().derived_key).toBe('admin-key');
46+
});
47+
48+
it('prefers the credentials already on the doc over the fallback', () => {
49+
service.updateUser({ _id: 'org.couchdb.user:admin', name: 'admin', roles: [], derived_key: 'own-key' }).subscribe();
50+
51+
expect(putBody().derived_key).toBe('own-key');
52+
});
53+
54+
it('strips underscore prefixed roles and writes to the user document id', () => {
55+
service.updateUser({ _id: 'org.couchdb.user:ann', name: 'ann', roles: [ '_admin', 'learner' ] }).subscribe();
56+
57+
expect(couchService.put.mock.calls[0][0]).toBe('_users/org.couchdb.user:ann');
58+
expect(putBody().roles).toEqual([ 'learner' ]);
59+
});
60+
});
61+
62+
});

src/app/shared/user.service.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,8 +216,14 @@ export class UserService {
216216

217217
updateUser(userInfo) {
218218
const planetConfiguration = this.stateService.configuration;
219+
// TEMP NOTE (for review, strip before merge): `credentials` holds the logged in user's own
220+
// password fields (derived_key, salt, ...). They were spread onto every update, so an admin
221+
// saving a copy of someone else's doc that had those fields stripped -- which is what the
222+
// profile view hands out -- replaced that user's password hash with the admin's and locked
223+
// them out. The fallback only makes sense for the editor's own doc.
224+
const isCurrentUser = userInfo._id !== undefined && userInfo._id === this.user._id;
219225
const newUserInfo = {
220-
...this.credentials,
226+
...(isCurrentUser ? this.credentials : {}),
221227
...userInfo,
222228
// Fix for Health & Achievements forms which can initialize middle name to undefined
223229
middleName: userInfo.middleName || '',

src/app/teams/teams-view.component.spec.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,25 @@ describe('TeamsViewComponent task projections', () => {
2222
expect(component.taskCount).toBe(1);
2323
});
2424

25+
// TEMP NOTE (for review, strip before merge): links are leaders only for now, and they live
26+
// on the global user doc, so the team leader is deliberately not an editor here.
27+
it('limits link editing to local leaders, edited by themselves or a planet admin', () => {
28+
const component: any = Object.create(TeamsViewComponent.prototype);
29+
component.planetCode = 'planet-a';
30+
component.currentUserId = 'alex';
31+
component.user = { _id: 'alex', isUserAdmin: false };
32+
const leaderDoc = { roles: [ 'leader' ] };
33+
34+
expect(component.canEditMemberLinks({ userId: 'alex', userPlanetCode: 'planet-a', userDoc: { doc: leaderDoc } })).toBe(true);
35+
expect(component.canEditMemberLinks({ userId: 'alex', userPlanetCode: 'planet-a', userDoc: { doc: { roles: [] } } })).toBe(false);
36+
expect(component.canEditMemberLinks({ userId: 'alex', userPlanetCode: 'planet-b', userDoc: { doc: leaderDoc } })).toBe(false);
37+
expect(component.canEditMemberLinks({ userId: 'bob', userPlanetCode: 'planet-a', userDoc: { doc: leaderDoc } })).toBe(false);
38+
39+
component.user = { _id: 'alex', isUserAdmin: true };
40+
41+
expect(component.canEditMemberLinks({ userId: 'bob', userPlanetCode: 'planet-a', userDoc: { doc: leaderDoc } })).toBe(true);
42+
});
43+
2544
it('matches membership on the local planet code rather than the user doc planet', () => {
2645
const component: any = Object.create(TeamsViewComponent.prototype);
2746
component.planetCode = 'planet-a';

src/app/teams/teams-view.component.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { Subject, forkJoin, of, throwError } from 'rxjs';
66
import { takeUntil, switchMap, finalize, map, tap, catchError } from 'rxjs/operators';
77
import { CouchService } from '../shared/couchdb.service';
88
import { UsersLinksService } from '../users/users-links.service';
9+
import { canHoldMemberLinks } from '../shared/social-platforms.constants';
910
import { DialogsPromptComponent } from '../shared/dialogs/dialogs-prompt.component';
1011
import { UserService } from '../shared/user.service';
1112
import { PlanetMessageService } from '../shared/planet-message.service';
@@ -462,6 +463,7 @@ export class TeamsViewComponent implements OnInit, AfterViewChecked, OnDestroy {
462463
// local planet admins rather than the team leader, who only manages team level fields.
463464
canEditMemberLinks(member): boolean {
464465
return member?.userPlanetCode === this.planetCode &&
466+
canHoldMemberLinks(member?.userDoc?.doc) &&
465467
(member?.userId === this.currentUserId || this.user.isUserAdmin === true);
466468
}
467469

src/app/users/users-profile/users-profile.component.html

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,11 @@ <h4 class="primary-text-color" matListItemTitle i18n>Full Name</h4>
7777
<mat-divider></mat-divider>
7878
<mat-list-item>
7979
<h4 class="primary-text-color" matListItemTitle i18n>Email</h4>
80-
<p matListItemLine><b>{{userDetail.email || 'N/A'}}</b></p>
80+
@if (canSeeContact('email')) {
81+
<p matListItemLine><b>{{userDetail.email || 'N/A'}}</b>@if (!isContactShared('email')) {<span class="contact-private" i18n>not shown to others</span>}</p>
82+
} @else {
83+
<p matListItemLine><b i18n>Not shared</b></p>
84+
}
8185
</mat-list-item>
8286
<mat-divider></mat-divider>
8387
<mat-list-item>
@@ -104,7 +108,11 @@ <h4 class="primary-text-color" matListItemTitle i18n>DOB</h4>
104108
<mat-divider></mat-divider>
105109
<mat-list-item>
106110
<h4 class="primary-text-color" matListItemTitle i18n>Phone</h4>
107-
<p matListItemLine><b>{{userDetail.phoneNumber || 'N/A'}}</b></p>
111+
@if (canSeeContact('phoneNumber')) {
112+
<p matListItemLine><b>{{userDetail.phoneNumber || 'N/A'}}</b>@if (!isContactShared('phoneNumber')) {<span class="contact-private" i18n>not shown to others</span>}</p>
113+
} @else {
114+
<p matListItemLine><b i18n>Not shared</b></p>
115+
}
108116
</mat-list-item>
109117
<mat-divider></mat-divider>
110118
<mat-list-item>
@@ -125,23 +133,25 @@ <h4 class="primary-text-color" matListItemTitle i18n>Member Since</h4>
125133
</mat-list>
126134
</div>
127135
<div class="cards-container">
128-
<mat-card>
129-
<mat-card-header>
130-
<mat-card-title i18n class="primary-text-color">Links &amp; Contacts</mat-card-title>
131-
</mat-card-header>
132-
<mat-card-content>
133-
@if (socialLinks.length > 0) {
134-
<planet-social-links [links]="socialLinks"></planet-social-links>
135-
} @else {
136-
<span i18n>No links added.</span>
137-
}
138-
@if (editable) {
139-
<button mat-stroked-button (click)="openLinksDialog()">
140-
<mat-icon>link</mat-icon><span i18n>{ hasLinks, select, true {Edit Links} false {Add Links}}</span>
141-
</button>
142-
}
143-
</mat-card-content>
144-
</mat-card>
136+
@if (socialLinks.length > 0 || canEditLinks) {
137+
<mat-card>
138+
<mat-card-header>
139+
<mat-card-title i18n class="primary-text-color">Links &amp; Contacts</mat-card-title>
140+
</mat-card-header>
141+
<mat-card-content>
142+
@if (socialLinks.length > 0) {
143+
<planet-social-links [links]="socialLinks"></planet-social-links>
144+
} @else {
145+
<span i18n>No links added.</span>
146+
}
147+
@if (canEditLinks) {
148+
<button mat-stroked-button (click)="openLinksDialog()">
149+
<mat-icon>link</mat-icon><span i18n>{ hasLinks, select, true {Edit Links} false {Add Links}}</span>
150+
</button>
151+
}
152+
</mat-card-content>
153+
</mat-card>
154+
}
145155
<mat-card>
146156
<mat-card-header>
147157
<mat-card-title i18n class="primary-text-color">Teams</mat-card-title>

src/app/users/users-profile/users-profile.component.spec.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,44 @@ describe('UserProfileComponent', () => {
4949
expect(backButton.nativeElement.getAttribute('aria-label')).toBe('Go back');
5050
});
5151

52+
// TEMP NOTE (for review, strip before merge): contacts are opt in now.
53+
it('hides contacts from other members until their owner shares them', () => {
54+
component.editable = false;
55+
component.userDetail = { name: 'member', email: 'member@ole.org', phoneNumber: '555' };
56+
57+
expect(component.canSeeContact('email')).toBe(false);
58+
expect(component.canSeeContact('phoneNumber')).toBe(false);
59+
60+
component.userDetail = { ...component.userDetail, contactVisibility: { email: true, phoneNumber: false } };
61+
62+
expect(component.canSeeContact('email')).toBe(true);
63+
expect(component.canSeeContact('phoneNumber')).toBe(false);
64+
});
65+
66+
it('keeps contacts visible to the member themselves and to planet admins', () => {
67+
component.editable = true;
68+
component.userDetail = { name: 'member', email: 'member@ole.org' };
69+
70+
expect(component.canSeeContact('email')).toBe(true);
71+
expect(component.isContactShared('email')).toBe(false);
72+
});
73+
74+
// TEMP NOTE (for review, strip before merge): links are leaders only for now.
75+
it('offers the link editor only on a leader profile the viewer can edit', () => {
76+
component.editable = true;
77+
component.userDetail = { name: 'member', roles: [] };
78+
79+
expect(component.canEditLinks).toBe(false);
80+
81+
component.userDetail = { name: 'member', roles: [ 'leader' ] };
82+
83+
expect(component.canEditLinks).toBe(true);
84+
85+
component.editable = false;
86+
87+
expect(component.canEditLinks).toBe(false);
88+
});
89+
5290
it('should not render toolbar back button when isDialog is true', () => {
5391
component.isDialog = true;
5492
fixture.detectChanges();

src/app/users/users-profile/users-profile.component.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { TruncateTextPipe } from '../../shared/truncate-text.pipe';
2525
import { AvatarComponent } from '../../shared/avatar.component';
2626
import { FullNamePipe } from '../../shared/full-name.pipe';
2727
import { SocialLinksComponent } from '../../shared/social-links.component';
28-
import { MemberLink, sanitizeMemberLinks } from '../../shared/social-platforms.constants';
28+
import { canHoldMemberLinks, MemberLink, sanitizeMemberLinks } from '../../shared/social-platforms.constants';
2929
import { UsersLinksService } from '../users-links.service';
3030

3131
@Component({
@@ -132,6 +132,22 @@ export class UsersProfileComponent implements OnInit, OnDestroy {
132132
return this.socialLinks.length > 0 ? 'true' : 'false';
133133
}
134134

135+
get canEditLinks(): boolean {
136+
return this.editable && canHoldMemberLinks(this.userDetail);
137+
}
138+
139+
// TEMP NOTE (for review, strip before merge): email and phone used to be readable by every
140+
// logged in user, including a nation browsing a community's leaders. They are opt in now, and
141+
// absent flags on existing docs read as opted out. `editable` is the member themselves or a
142+
// local planet admin, who both keep the old view.
143+
isContactShared(field: 'email' | 'phoneNumber'): boolean {
144+
return this.userDetail?.contactVisibility?.[field] === true;
145+
}
146+
147+
canSeeContact(field: 'email' | 'phoneNumber'): boolean {
148+
return this.editable || this.isContactShared(field);
149+
}
150+
135151
openLinksDialog() {
136152
this.usersLinksService.openDialog(this.userDetail.name, this.socialLinks).pipe(
137153
takeUntil(this.onDestroy$)

0 commit comments

Comments
 (0)