Skip to content

Commit

Permalink
[MM-62701] [MM-62176]Edit custom profile attributes in user profile (#…
Browse files Browse the repository at this point in the history
…8557)

* feat: Add support for custom profile attributes in edit profile form

feat: Add support for custom profile attributes in edit profile

refactor: Normalize whitespace in CustomAttribute type definition

fix: Resolve type mismatch for customAttributes in UserInfo interface

test: Add test for udpateCustomProfileAttributeValues method

fix typing, submit changes to server

missing files

test: Add tests for CustomProfileField component

fix naming

fix imports

fix

feat: Add field_refs hook for managing field references

feat: Make `setRef` ref parameter optional with default no-op implementation

refactor: Replace CustomProfileField with useFieldRefs for profile form

refactor: Optimize edit profile screen imports and custom attributes handling

refactor: Move custom attributes logic to remote actions in user.ts

address PR reviews

test: Add tests for custom attributes in edit profile

test: Add tests for EditProfile component with custom attributes

fix: Add UserModel type assertion to currentUser in edit profile tests

test: Add tests for ProfileForm custom attributes functionality

test: Add comprehensive tests for useFieldRefs hook

test: Add tests for fetchCustomAttributes and updateCustomAttributes

add tests

remove unneeded files

review changes

remove counter from hook

remove package.resolved

create interface for reuse of record

* fix signature type
  • Loading branch information
Willyfrog authored and Rajat-Dabade committed Feb 25, 2025
1 parent 8e02e06 commit bcc4259
Show file tree
Hide file tree
Showing 15 changed files with 706 additions and 135 deletions.
113 changes: 112 additions & 1 deletion app/actions/remote/user.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ import {
autoUpdateTimezone,
fetchTeamAndChannelMembership,
getAllSupportedTimezones,
fetchCustomAttributes,
updateCustomAttributes,
} from './user';

import type ServerDataOperator from '@database/operator/server_data_operator';
Expand Down Expand Up @@ -90,6 +92,9 @@ const mockClient = {
getTeamMember: jest.fn((id: string, userId: string) => ({id: userId + '-' + id, user_id: userId, team_id: id, roles: ''})),
getChannelMember: jest.fn((cid: string, userId: string) => ({id: userId + '-' + cid, user_id: userId, channel_id: cid, roles: ''})),
getTimezones: jest.fn(() => ['EST']),
getCustomProfileAttributeFields: jest.fn(),
getCustomProfileAttributeValues: jest.fn(),
updateCustomProfileAttributeValues: jest.fn(),
};

beforeAll(() => {
Expand Down Expand Up @@ -337,7 +342,7 @@ describe('get users', () => {
});

it('buildProfileImageUrlFromUser - base case', async () => {
const result = await buildProfileImageUrlFromUser(serverUrl, user2);
const result = buildProfileImageUrlFromUser(serverUrl, user2);
expect(result).toBeDefined();
});

Expand All @@ -358,6 +363,112 @@ describe('get users', () => {
});
});

describe('Custom Profile Attributes', () => {
it('fetchCustomAttributes - base case', async () => {
mockClient.getCustomProfileAttributeFields = jest.fn().mockResolvedValue([
{id: 'field1', name: 'Field 1'},
{id: 'field2', name: 'Field 2'},
]);
mockClient.getCustomProfileAttributeValues = jest.fn().mockResolvedValue({
field1: 'value1',
field2: 'value2',
});

const result = await fetchCustomAttributes(serverUrl, 'user1');
expect(result.error).toBeUndefined();
expect(result.attributes).toBeDefined();
expect(Object.keys(result.attributes!)).toHaveLength(2);
expect(result.attributes!.field1).toEqual({
id: 'field1',
name: 'Field 1',
value: 'value1',
});
expect(result.attributes!.field2).toEqual({
id: 'field2',
name: 'Field 2',
value: 'value2',
});
});

it('fetchCustomAttributes - no fields', async () => {
mockClient.getCustomProfileAttributeFields = jest.fn().mockResolvedValue([]);
mockClient.getCustomProfileAttributeValues = jest.fn().mockResolvedValue({});

const result = await fetchCustomAttributes(serverUrl, 'user1');
expect(result.error).toBeUndefined();
expect(result.attributes).toEqual({});
});

it('fetchCustomAttributes - error on fields', async () => {
const error = new Error('Sample error');

mockClient.getCustomProfileAttributeFields = jest.fn().mockRejectedValue(error);
mockClient.getCustomProfileAttributeValues = jest.fn().mockResolvedValue({
field1: 'value1',
field2: 'value2',
});

const result = await fetchCustomAttributes(serverUrl, 'user1');
expect(result.error).toBeDefined();
});

it('fetchCustomAttributes - error on values', async () => {
const error = new Error('Sample error');

mockClient.getCustomProfileAttributeFields = jest.fn().mockResolvedValue([
{id: 'field1', name: 'Field 1'},
{id: 'field2', name: 'Field 2'},
]);
mockClient.getCustomProfileAttributeValues = jest.fn().mockRejectedValue(error);

const result = await fetchCustomAttributes(serverUrl, 'user1');
expect(result.error).toBeDefined();
});

it('updateCustomAttributes - base case', async () => {
mockClient.updateCustomProfileAttributeValues = jest.fn().mockResolvedValue({});

const attributes = {
field1: {
id: 'field1',
name: 'Field 1',
value: 'new value 1',
},
field2: {
id: 'field2',
name: 'Field 2',
value: 'new value 2',
},
};

const result = await updateCustomAttributes(serverUrl, attributes);
expect(result.error).toBeUndefined();
expect(result.success).toBe(true);
expect(mockClient.updateCustomProfileAttributeValues).toHaveBeenCalledWith({
field1: 'new value 1',
field2: 'new value 2',
});
});

it('updateCustomAttributes - error', async () => {
const error = new Error('Test Error');

mockClient.updateCustomProfileAttributeValues = jest.fn().mockRejectedValue(error);

const attributes = {
field1: {
id: 'field1',
name: 'Field 1',
value: 'new value 1',
},
};

const result = await updateCustomAttributes(serverUrl, attributes);
expect(result.error).toBeDefined();
expect(result.success).toBe(false);
});
});

describe('update users', () => {
it('updateMe - handle not found database', async () => {
const result = await updateMe('foo', {});
Expand Down
44 changes: 44 additions & 0 deletions app/actions/remote/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {forceLogoutIfNecessary} from './session';

import type {Model} from '@nozbe/watermelondb';
import type UserModel from '@typings/database/models/servers/user';
import type {CustomAttribute, CustomAttributeSet} from '@typings/screens/edit_profile';

export type MyUserRequest = {
user?: UserProfile;
Expand Down Expand Up @@ -878,3 +879,46 @@ export const getAllSupportedTimezones = async (serverUrl: string) => {
return [];
}
};

export const fetchCustomAttributes = async (serverUrl: string, userId: string): Promise<{attributes: CustomAttributeSet; error: unknown}> => {
try {
const client = NetworkManager.getClient(serverUrl);
const [fields, attrValues] = await Promise.all([
client.getCustomProfileAttributeFields(),
client.getCustomProfileAttributeValues(userId),
]);

if (fields?.length > 0) {
const attributes: Record<string, CustomAttribute> = {};
fields.forEach((field) => {
attributes[field.id] = {
id: field.id,
name: field.name,
value: attrValues[field.id] || '',
};
});
return {attributes, error: undefined};
}
return {attributes: {} as Record<string, CustomAttribute>, error: undefined};
} catch (error) {
logDebug('error on fetchCustomAttributes', getFullErrorMessage(error));
forceLogoutIfNecessary(serverUrl, error);
return {attributes: {} as Record<string, CustomAttribute>, error};
}
};

export const updateCustomAttributes = async (serverUrl: string, attributes: CustomAttributeSet): Promise<{success: boolean; error: unknown}> => {
try {
const client = NetworkManager.getClient(serverUrl);
const values: CustomProfileAttributeSimple = {};
Object.keys(attributes).forEach((field) => {
values[field] = attributes[field].value;
});
await client.updateCustomProfileAttributeValues(values);
return {success: true, error: undefined};
} catch (error) {
logDebug('error on updateCustomAttributes', getFullErrorMessage(error));
forceLogoutIfNecessary(serverUrl, error);
return {error, success: false};
}
};
4 changes: 4 additions & 0 deletions app/client/rest/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,10 @@ export default class ClientBase extends ClientTracking {
return `${this.getPostsRoute()}/schedule`;
}

getUserCustomProfileAttributesRoute(userId: string) {
return `${this.getUsersRoute()}/${userId}/custom_profile_attributes`;
}

doFetch = async (url: string, options: ClientOptions, returnDataOnly = true) => {
return this.doFetchWithTracking(url, options, returnDataOnly);
};
Expand Down
16 changes: 16 additions & 0 deletions app/client/rest/custom_profile_attributes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,20 @@ describe('CustomAttributes', () => {
{method: 'get'},
);
});

test('updateCustomProfileAttributeValues', async () => {
const values = {
field_1: 'value1',
field_2: 'value2',
};
await client.updateCustomProfileAttributeValues(values);

expect(client.doFetch).toHaveBeenCalledWith(
`${client.getCustomProfileAttributesRoute()}/values`,
{
method: 'patch',
body: values,
},
);
});
});
12 changes: 11 additions & 1 deletion app/client/rest/custom_profile_attributes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type ClientBase from './base';
export interface ClientCustomAttributesMix {
getCustomProfileAttributeFields: () => Promise<CustomProfileField[]>;
getCustomProfileAttributeValues: (userID: string) => Promise<CustomProfileAttributeSimple>;
updateCustomProfileAttributeValues: (values: CustomProfileAttributeSimple) => Promise<string>;
}

const ClientCustomAttributes = <TBase extends Constructor<ClientBase>>(superclass: TBase) => class extends superclass {
Expand All @@ -18,10 +19,19 @@ const ClientCustomAttributes = <TBase extends Constructor<ClientBase>>(superclas

getCustomProfileAttributeValues = async (userID: string) => {
return this.doFetch(
`${this.getUserRoute(userID)}/custom_profile_attributes`,
`${this.getUserCustomProfileAttributesRoute(userID)}`,
{method: 'get'},
);
};
updateCustomProfileAttributeValues = async (values: CustomProfileAttributeSimple) => {
return this.doFetch(
`${this.getCustomProfileAttributesRoute()}/values`,
{
method: 'patch',
body: values,
},
);
};
};

export default ClientCustomAttributes;
1 change: 1 addition & 0 deletions app/components/floating_text_input_label/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ const FloatingTextInput = forwardRef<FloatingTextInputRef, FloatingTextInputProp
}

return res;
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [styles, theme, shouldShowError, focused, textInputStyle, focusedLabel, multiline, multilineInputHeight, editable]);

const textAnimatedTextStyle = useAnimatedStyle(() => {
Expand Down
103 changes: 103 additions & 0 deletions app/hooks/field_refs.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

import {renderHook, act} from '@testing-library/react-hooks';

import useFieldRefs from './field_refs';

describe('useFieldRefs', () => {
it('should initialize with empty refs', () => {
const {result} = renderHook(() => useFieldRefs());
const [getRef] = result.current;

expect(getRef('test')).toBeUndefined();
});

it('should set and get refs', () => {
const {result} = renderHook(() => useFieldRefs());
const [getRef, setRef] = result.current;

const mockRef = {
blur: jest.fn(),
focus: jest.fn(),
isFocused: jest.fn(),
};

act(() => {
setRef('testField')(mockRef);
});

expect(getRef('testField')).toBe(mockRef);
});

it('should track number of refs', () => {
const {result} = renderHook(() => useFieldRefs());
const [, setRef] = result.current;

const mockRef1 = {
blur: jest.fn(),
focus: jest.fn(),
isFocused: jest.fn(),
};

const mockRef2 = {
blur: jest.fn(),
focus: jest.fn(),
isFocused: jest.fn(),
};

act(() => {
setRef('field1')(mockRef1);
setRef('field2')(mockRef2);
});
});

it('should remove refs when cleanup function is called', () => {
const {result} = renderHook(() => useFieldRefs());
const [getRef, setRef] = result.current;

const mockRef = {
blur: jest.fn(),
focus: jest.fn(),
isFocused: jest.fn(),
};

let cleanup: () => void;
act(() => {
cleanup = setRef('testField')(mockRef);
});

expect(getRef('testField')).toBe(mockRef);

act(() => {
cleanup!();
});

expect(getRef('testField')).toBeUndefined();
});

it('should handle multiple refs independently', () => {
const {result} = renderHook(() => useFieldRefs());
const [getRef, setRef] = result.current;

const mockRef1 = {
blur: jest.fn(),
focus: jest.fn(),
isFocused: jest.fn(),
};

const mockRef2 = {
blur: jest.fn(),
focus: jest.fn(),
isFocused: jest.fn(),
};

act(() => {
setRef('field1')(mockRef1);
setRef('field2')(mockRef2);
});

expect(getRef('field1')).toBe(mockRef1);
expect(getRef('field2')).toBe(mockRef2);
});
});
Loading

0 comments on commit bcc4259

Please sign in to comment.