Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions src/libs/actions/Policy/Tag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
import type {OnyxData} from '@src/types/onyx/Request';

let allPolicyTags: OnyxCollection<PolicyTagLists> = {};
Onyx.connect({

Check warning on line 38 in src/libs/actions/Policy/Tag.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.POLICY_TAGS,
waitForCollectionCallback: true,
callback: (value) => {
Expand Down Expand Up @@ -653,7 +653,13 @@
API.write(WRITE_COMMANDS.RENAME_POLICY_TAG, parameters, onyxData);
}

function enablePolicyTags(policyID: string, enabled: boolean) {
type EnablePolicyTagsProps = {
policyID: string;
enabled: boolean;
policyTags?: PolicyTagLists;
};

function enablePolicyTags({policyID, enabled, policyTags}: EnablePolicyTagsProps) {
const onyxData: OnyxData = {
optimisticData: [
{
Expand Down Expand Up @@ -692,8 +698,7 @@
],
};

const policyTagList = allPolicyTags?.[`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`];
if (!policyTagList) {
if (!policyTags) {
const defaultTagList: PolicyTagLists = {
Tag: {
name: 'Tag',
Expand All @@ -713,7 +718,7 @@
value: null,
});
} else if (!enabled) {
const policyTag = PolicyUtils.getTagLists(policyTagList).at(0);
const policyTag = PolicyUtils.getTagLists(policyTags).at(0);

if (!policyTag) {
return;
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/WorkspaceMoreFeaturesPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ function WorkspaceMoreFeaturesPage({policy, route}: WorkspaceMoreFeaturesPagePro
if (!policyID) {
return;
}
enablePolicyTags(policyID, isEnabled);
enablePolicyTags({policyID, enabled: isEnabled, policyTags: policyTagLists});
},
},
{
Expand Down
149 changes: 148 additions & 1 deletion tests/actions/PolicyTagTest.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {renderHook, waitFor} from '@testing-library/react-native';
import {act, renderHook, waitFor} from '@testing-library/react-native';
import Onyx from 'react-native-onyx';
import OnyxUtils from 'react-native-onyx/dist/OnyxUtils';
import useOnyx from '@hooks/useOnyx';
Expand All @@ -9,6 +9,7 @@ import {
clearPolicyTagListErrorField,
createPolicyTag,
deletePolicyTags,
enablePolicyTags,
renamePolicyTag,
renamePolicyTagList,
setPolicyRequiresTag,
Expand Down Expand Up @@ -1382,4 +1383,150 @@ describe('actions/Policy', () => {
});
});
});

describe('EnablePolicyTags', () => {
it('should enable tags and create default tag list if none exists', async () => {
// Given a policy without tags
const fakePolicy = createRandomPolicy(0);
fakePolicy.areTagsEnabled = false;

mockFetch.pause();

await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy);

// When enabling tags
enablePolicyTags({policyID: fakePolicy.id, enabled: true});
await waitForBatchedUpdates();

// Then the policy should be updated optimistically
const optimisticPolicy = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`);
expect(optimisticPolicy?.areTagsEnabled).toBe(true);
expect(optimisticPolicy?.pendingFields?.areTagsEnabled).toBe(CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE);

// And a default tag list should be created
const optimisticPolicyTags = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`);
expect(optimisticPolicyTags?.Tag?.name).toBe('Tag');
expect(optimisticPolicyTags?.Tag?.orderWeight).toBe(0);
expect(optimisticPolicyTags?.Tag?.required).toBe(false);
expect(optimisticPolicyTags?.Tag?.tags).toEqual({});

mockFetch.resume();
await waitForBatchedUpdates();

// And after API success, pending fields should be cleared
const successPolicy = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`);
expect(successPolicy?.pendingFields?.areTagsEnabled).toBeFalsy();
Comment on lines +1416 to +1418

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I would also add checking the data from the optimistic update also after success, because we have no guarantee that someone will not accidentally overwrite something in the success state.

Suggested change
// And after API success, pending fields should be cleared
const successPolicy = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`);
expect(successPolicy?.pendingFields?.areTagsEnabled).toBeFalsy();
// And after API success, pending fields should be cleared
const successPolicy = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`);
expect(successPolicy?.pendingFields?.areTagsEnabled).toBeFalsy();
// And a default tag list should be created
const optimisticPolicyTags = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`);
expect(optimisticPolicyTags?.Tag?.name).toBe('Tag');
expect(optimisticPolicyTags?.Tag?.orderWeight).toBe(0);
expect(optimisticPolicyTags?.Tag?.required).toBe(false);
expect(optimisticPolicyTags?.Tag?.tags).toEqual({});

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it's worth adding here. I don't see any other test with such a check

});

it('should disable tags and update existing tag list', async () => {
// Given a policy with enabled tags and existing tag list
const fakePolicy = createRandomPolicy(0);
fakePolicy.areTagsEnabled = true;

const tagListName = 'Tag';
const fakePolicyTags = createRandomPolicyTags(tagListName, 2);
const existingTags = fakePolicyTags[tagListName]?.tags ?? {};

mockFetch.pause();

await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy);
await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, fakePolicyTags);

// When disabling tags
enablePolicyTags({policyID: fakePolicy.id, enabled: false, policyTags: fakePolicyTags});
await waitForBatchedUpdates();

// Then the policy should be updated optimistically
const optimisticPolicy = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`);
expect(optimisticPolicy?.areTagsEnabled).toBe(false);
expect(optimisticPolicy?.requiresTag).toBe(false);
expect(optimisticPolicy?.pendingFields?.areTagsEnabled).toBe(CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE);

// And all tags should be disabled
const optimisticPolicyTags = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`);
Object.keys(existingTags).forEach((tagName) => {
expect(optimisticPolicyTags?.[tagListName]?.tags[tagName]?.enabled).toBe(false);
});

mockFetch.resume();
await waitForBatchedUpdates();

// And after API success, pending fields should be cleared
const successPolicy = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`);
expect(successPolicy?.pendingFields?.areTagsEnabled).toBeFalsy();
});

it('should reset changes when API returns error', async () => {
// Given a policy with disabled tags
const fakePolicy = createRandomPolicy(0);
fakePolicy.areTagsEnabled = false;

mockFetch.pause();

await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy);

mockFetch.fail();

// When enabling tags fails
enablePolicyTags({policyID: fakePolicy.id, enabled: true});
await waitForBatchedUpdates();

mockFetch.resume();
await waitForBatchedUpdates();

// Then the policy should be reset to original state
const failurePolicy = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`);
expect(failurePolicy?.areTagsEnabled).toBe(false);
expect(failurePolicy?.pendingFields?.areTagsEnabled).toBeFalsy();

// And no tag list should be created
const failurePolicyTags = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`);
expect(failurePolicyTags).toBeFalsy();
});

it('should work with data from useOnyx hook', async () => {
// Given a policy data loaded via useOnyx
const fakePolicy = createRandomPolicy(0);
fakePolicy.areTagsEnabled = false;

mockFetch.pause();

await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy);

const {result} = renderHook(() => {
const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, {canBeMissing: true});
const [policyTags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, {canBeMissing: true});
return {policy, policyTags};
});

await waitFor(() => {
expect(result.current.policy).toBeDefined();
});

// When enabling tags with data from useOnyx
await act(async () => {
enablePolicyTags({policyID: fakePolicy.id, enabled: true, policyTags: result.current.policyTags});
await waitForBatchedUpdates();
});

// Then the policy should be updated optimistically
const optimisticPolicy = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`);
expect(optimisticPolicy?.areTagsEnabled).toBe(true);

await act(async () => {
await mockFetch.resume();
await waitForBatchedUpdates();
});

// And after API success, policy should be enabled
await waitFor(() => {
expect(result.current.policy?.areTagsEnabled).toBe(true);
});

// And default tag list should be created
await waitFor(() => {
expect(result.current.policyTags?.Tag).toBeDefined();
});
});
});
});
Loading