Skip to content

Commit 473c358

Browse files
authored
refactor(mobile): guard add reviewer while pending, parameterise tests
Address review feedback on the suggested reviewers UI: - Disable the "Add" button and short-circuit toggleReviewer while a mutation is in-flight so overlapping optimistic updates can't flicker or roll back to a stale snapshot. - Convert toSuggestedReviewerWriteContent and reviewerMatchesAvailable tests to it.each parameterised cases per project convention. Generated-By: PostHog Code Task-Id: 4b8dd76e-7ba9-453b-8cb8-c9441a47b3dc
1 parent 98f9404 commit 473c358

2 files changed

Lines changed: 48 additions & 36 deletions

File tree

apps/mobile/src/features/inbox/components/SuggestedReviewers.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ export function SuggestedReviewers({
7575
};
7676

7777
const toggleReviewer = (option: AvailableSuggestedReviewer) => {
78+
if (isPending) return;
7879
const existing = reviewers.find((r) => reviewerMatchesAvailable(r, option));
7980
if (existing) {
8081
removeReviewer(existing);
@@ -119,9 +120,10 @@ export function SuggestedReviewers({
119120
<View className="flex-1" />
120121
<Pressable
121122
onPress={() => setEditOpen(true)}
123+
disabled={isPending}
122124
accessibilityLabel="Add suggested reviewer"
123125
hitSlop={6}
124-
className="flex-row items-center gap-1 rounded-full border border-gray-6 px-2.5 py-1 active:opacity-70"
126+
className="flex-row items-center gap-1 rounded-full border border-gray-6 px-2.5 py-1 active:opacity-70 disabled:opacity-50"
125127
>
126128
<Plus size={12} color={themeColors.gray[11]} weight="bold" />
127129
<Text className="text-[12px] text-gray-11">Add</Text>

apps/mobile/src/features/inbox/utils.test.ts

Lines changed: 45 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -171,51 +171,61 @@ describe("buildInboxViewedProperties", () => {
171171
});
172172

173173
describe("toSuggestedReviewerWriteContent", () => {
174-
it("prefers github_login so the server preserves commits/name", () => {
175-
const content = toSuggestedReviewerWriteContent([
176-
makeReviewer({
174+
it.each([
175+
{
176+
name: "prefers github_login so the server preserves commits/name",
177+
reviewer: makeReviewer({
177178
github_login: "ada",
178179
user: { id: 1, uuid: "u1", email: "", first_name: "", last_name: "" },
179180
}),
180-
]);
181-
expect(content).toEqual([{ github_login: "ada" }]);
182-
});
183-
184-
it("falls back to user_uuid when there is no github_login", () => {
185-
const content = toSuggestedReviewerWriteContent([
186-
makeReviewer({
181+
expected: [{ github_login: "ada" }],
182+
},
183+
{
184+
name: "falls back to user_uuid when there is no github_login",
185+
reviewer: makeReviewer({
187186
github_login: "",
188187
user: { id: 1, uuid: "u1", email: "", first_name: "", last_name: "" },
189188
}),
190-
]);
191-
expect(content).toEqual([{ user_uuid: "u1" }]);
192-
});
193-
194-
it("drops entries with neither a login nor a resolved user", () => {
195-
const content = toSuggestedReviewerWriteContent([
196-
makeReviewer({ github_login: "", user: null }),
197-
]);
198-
expect(content).toEqual([]);
189+
expected: [{ user_uuid: "u1" }],
190+
},
191+
{
192+
name: "drops entries with neither a login nor a resolved user",
193+
reviewer: makeReviewer({ github_login: "", user: null }),
194+
expected: [],
195+
},
196+
])("$name", ({ reviewer, expected }) => {
197+
expect(toSuggestedReviewerWriteContent([reviewer])).toEqual(expected);
199198
});
200199
});
201200

202201
describe("reviewerMatchesAvailable", () => {
203-
it("matches on user uuid", () => {
204-
const reviewer = makeReviewer({
205-
github_login: "",
206-
user: { id: 1, uuid: "uuid-1", email: "", first_name: "", last_name: "" },
207-
});
208-
expect(reviewerMatchesAvailable(reviewer, makeAvailable())).toBe(true);
209-
});
210-
211-
it("matches on case-insensitive github login", () => {
212-
const reviewer = makeReviewer({ github_login: "ADA", user: null });
213-
expect(reviewerMatchesAvailable(reviewer, makeAvailable())).toBe(true);
214-
});
215-
216-
it("does not match different people", () => {
217-
const reviewer = makeReviewer({ github_login: "octocat", user: null });
218-
expect(reviewerMatchesAvailable(reviewer, makeAvailable())).toBe(false);
202+
it.each([
203+
{
204+
name: "matches on user uuid",
205+
reviewer: makeReviewer({
206+
github_login: "",
207+
user: {
208+
id: 1,
209+
uuid: "uuid-1",
210+
email: "",
211+
first_name: "",
212+
last_name: "",
213+
},
214+
}),
215+
expected: true,
216+
},
217+
{
218+
name: "matches on case-insensitive github login",
219+
reviewer: makeReviewer({ github_login: "ADA", user: null }),
220+
expected: true,
221+
},
222+
{
223+
name: "does not match different people",
224+
reviewer: makeReviewer({ github_login: "octocat", user: null }),
225+
expected: false,
226+
},
227+
])("$name", ({ reviewer, expected }) => {
228+
expect(reviewerMatchesAvailable(reviewer, makeAvailable())).toBe(expected);
219229
});
220230
});
221231

0 commit comments

Comments
 (0)