Skip to content

Commit c60b912

Browse files
Oba-OneOba-One
authored
Merge pull request #31 from greenpill-dev-guild/codex/review-feedback-fixes
fix: address review feedback across client and contracts
2 parents d955e2b + cde3c3a commit c60b912

22 files changed

+636
-154
lines changed

‎client/__tests__/MetadataParser.test.tsx‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -74,15 +74,15 @@ describe("parseMetadata", () => {
7474

7575
render(<MetadataTest metadata={metadata} />);
7676
expect(screen.getByTestId("name")).toHaveTextContent("My Special Jar");
77-
expect(screen.getByTestId("description")).toHaveTextContent(
78-
"A detailed description",
79-
);
80-
expect(screen.getByTestId("image")).toHaveAttribute(
81-
"src",
82-
expect.stringContaining(
83-
encodeURIComponent("https://example.com/image.png"),
84-
),
85-
);
77+
expect(screen.getByTestId("description")).toHaveTextContent(
78+
"A detailed description",
79+
);
80+
expect(screen.getByTestId("image")).toHaveAttribute(
81+
"src",
82+
expect.stringContaining(
83+
encodeURIComponent("https://example.com/image.png"),
84+
),
85+
);
8686
expect(screen.getByTestId("link")).toHaveAttribute(
8787
"href",
8888
"https://example.com/project",
@@ -98,13 +98,13 @@ describe("parseMetadata", () => {
9898

9999
render(<MetadataTest metadata={metadata} />);
100100
expect(screen.getByTestId("name")).toHaveTextContent("Partial Jar");
101-
expect(screen.getByTestId("description")).toHaveTextContent(metadata); // fallback to raw string
102-
expect(screen.getByTestId("image")).toHaveAttribute(
103-
"src",
104-
expect.stringContaining(
105-
encodeURIComponent("https://example.com/image.png"),
106-
),
107-
);
101+
expect(screen.getByTestId("description")).toHaveTextContent(metadata); // fallback to raw string
102+
expect(screen.getByTestId("image")).toHaveAttribute(
103+
"src",
104+
expect.stringContaining(
105+
encodeURIComponent("https://example.com/image.png"),
106+
),
107+
);
108108
expect(screen.getByTestId("link")).toHaveAttribute("href", "");
109109
});
110110

@@ -154,15 +154,15 @@ describe("parseMetadata", () => {
154154

155155
render(<MetadataTest metadata={metadata} />);
156156
expect(screen.getByTestId("name")).toHaveTextContent("Complete Jar");
157-
expect(screen.getByTestId("description")).toHaveTextContent(
158-
"Full description",
159-
);
160-
expect(screen.getByTestId("image")).toHaveAttribute(
161-
"src",
162-
expect.stringContaining(
163-
encodeURIComponent("https://example.com/complete.png"),
164-
),
165-
);
157+
expect(screen.getByTestId("description")).toHaveTextContent(
158+
"Full description",
159+
);
160+
expect(screen.getByTestId("image")).toHaveAttribute(
161+
"src",
162+
expect.stringContaining(
163+
encodeURIComponent("https://example.com/complete.png"),
164+
),
165+
);
166166
expect(screen.getByTestId("link")).toHaveAttribute(
167167
"href",
168168
"https://complete-project.com",

‎client/__tests__/hooks/jar/createV2CreateArgs.test.ts‎

Lines changed: 240 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,13 @@ import {
55
FACTORY_DEFAULT_FEE_SENTINEL,
66
buildV2CreateCookieJarArgs,
77
getFeePercentageOnDeposit,
8+
getAccessConfigValidationError,
89
} from "@/hooks/jar/createV2CreateArgs";
9-
import { ETH_ADDRESS, HATS_PROTOCOL_ADDRESS, POAP_TOKEN_ADDRESS } from "@/lib/blockchain/constants";
10+
import {
11+
ETH_ADDRESS,
12+
HATS_PROTOCOL_ADDRESS,
13+
POAP_TOKEN_ADDRESS,
14+
} from "@/lib/blockchain/constants";
1015

1116
vi.mock("@/hooks/jar/schemas/jarCreationSchema", () => ({
1217
AccessType: {
@@ -169,7 +174,8 @@ describe("buildV2CreateCookieJarArgs", () => {
169174
expect(poapJar.accessType).toBe(1);
170175
expect(poapAccess.nftRequirement.nftContract).toBe(POAP_TOKEN_ADDRESS);
171176
expect(poapAccess.nftRequirement.tokenId).toBe(1234n);
172-
expect(poapAccess.nftRequirement.minBalance).toBe(1n);
177+
expect(poapAccess.nftRequirement.minBalance).toBe(0n);
178+
expect(poapAccess.nftRequirement.isPoapEventGate).toBe(true);
173179

174180
const [unlockJar] = buildV2CreateCookieJarArgs({
175181
values: makeValues({
@@ -202,12 +208,243 @@ describe("buildV2CreateCookieJarArgs", () => {
202208
const [hatsJar, hatsAccess] = buildV2CreateCookieJarArgs({
203209
values: makeValues({
204210
accessType: AccessType.Hats,
205-
protocolConfig: { accessType: "Hats", hatsId: 99 },
211+
protocolConfig: { accessType: "Hats", hatsId: "99" },
206212
}),
207213
metadata: "metadata",
208214
parseAmount: () => 1n,
209215
});
210216
expect(hatsJar.accessType).toBe(2);
211217
expect(hatsAccess.nftRequirement.nftContract).toBe(HATS_PROTOCOL_ADDRESS);
218+
expect(hatsAccess.nftRequirement.isPoapEventGate).toBe(false);
219+
});
220+
});
221+
222+
describe("getAccessConfigValidationError", () => {
223+
it("returns an error for NFT-gated access when address list is missing", () => {
224+
const error = getAccessConfigValidationError(
225+
makeValues({
226+
accessType: AccessType.NFTGated,
227+
nftAddresses: [],
228+
nftTypes: [],
229+
}),
230+
);
231+
expect(error).toBe("At least one NFT address is required for NFT-gated access");
232+
});
233+
234+
it("returns an error for NFT-gated access when nftAddresses and nftTypes mismatch", () => {
235+
const error = getAccessConfigValidationError(
236+
makeValues({
237+
accessType: AccessType.NFTGated,
238+
nftAddresses: [
239+
"0x1111111111111111111111111111111111111111",
240+
"0x2222222222222222222222222222222222222222",
241+
],
242+
nftTypes: [NFTType.ERC721],
243+
}),
244+
);
245+
expect(error).toBe("NFT addresses and NFT types must have the same length");
246+
});
247+
248+
it("returns an error for NFT-gated access when address format is invalid", () => {
249+
const error = getAccessConfigValidationError(
250+
makeValues({
251+
accessType: AccessType.NFTGated,
252+
nftAddresses: ["not-an-address"],
253+
nftTypes: [NFTType.ERC721],
254+
}),
255+
);
256+
expect(error).toBe("NFT address must be a valid Ethereum address");
257+
});
258+
259+
it("returns undefined for valid NFT-gated access", () => {
260+
const error = getAccessConfigValidationError(
261+
makeValues({
262+
accessType: AccessType.NFTGated,
263+
nftAddresses: ["0x1111111111111111111111111111111111111111"],
264+
nftTypes: [NFTType.ERC1155],
265+
}),
266+
);
267+
expect(error).toBeUndefined();
268+
});
269+
270+
it("returns an error for POAP access when eventId is missing", () => {
271+
const error = getAccessConfigValidationError(
272+
makeValues({
273+
accessType: AccessType.POAP,
274+
protocolConfig: { accessType: "POAP" },
275+
}),
276+
);
277+
expect(error).toBe("POAP event is required");
278+
});
279+
280+
it("returns an error for POAP access when eventId is non-numeric", () => {
281+
const error = getAccessConfigValidationError(
282+
makeValues({
283+
accessType: AccessType.POAP,
284+
protocolConfig: { accessType: "POAP", eventId: "abc" },
285+
}),
286+
);
287+
expect(error).toBe("POAP event must be a valid number");
288+
});
289+
290+
it("returns an error for POAP access when contract fallback is invalid", () => {
291+
const error = getAccessConfigValidationError(
292+
makeValues({
293+
accessType: AccessType.POAP,
294+
protocolConfig: {
295+
accessType: "POAP",
296+
eventId: "1234",
297+
poapContractAddress: "invalid-contract",
298+
},
299+
}),
300+
);
301+
expect(error).toBe("POAP contract address must be a valid Ethereum address");
302+
});
303+
304+
it("returns undefined for valid POAP access", () => {
305+
const error = getAccessConfigValidationError(
306+
makeValues({
307+
accessType: AccessType.POAP,
308+
protocolConfig: {
309+
accessType: "POAP",
310+
eventId: "1234",
311+
poapContractAddress: POAP_TOKEN_ADDRESS,
312+
},
313+
}),
314+
);
315+
expect(error).toBeUndefined();
316+
});
317+
318+
it("returns an error for Unlock access when unlockAddress is missing", () => {
319+
const error = getAccessConfigValidationError(
320+
makeValues({
321+
accessType: AccessType.Unlock,
322+
protocolConfig: { accessType: "Unlock" },
323+
}),
324+
);
325+
expect(error).toBe("Unlock contract address is required");
326+
});
327+
328+
it("returns an error for Unlock access when unlockAddress is invalid", () => {
329+
const error = getAccessConfigValidationError(
330+
makeValues({
331+
accessType: AccessType.Unlock,
332+
protocolConfig: { accessType: "Unlock", unlockAddress: "invalid" },
333+
}),
334+
);
335+
expect(error).toBe("Unlock contract address must be a valid Ethereum address");
336+
});
337+
338+
it("returns undefined for valid Unlock access", () => {
339+
const error = getAccessConfigValidationError(
340+
makeValues({
341+
accessType: AccessType.Unlock,
342+
protocolConfig: {
343+
accessType: "Unlock",
344+
unlockAddress: "0x2222222222222222222222222222222222222222",
345+
},
346+
}),
347+
);
348+
expect(error).toBeUndefined();
349+
});
350+
351+
it("returns an error for Hypercert access when address is missing", () => {
352+
const error = getAccessConfigValidationError(
353+
makeValues({
354+
accessType: AccessType.Hypercert,
355+
protocolConfig: {
356+
accessType: "Hypercert",
357+
hypercertTokenId: "1",
358+
},
359+
}),
360+
);
361+
expect(error).toBe("Hypercert contract address is required");
362+
});
363+
364+
it("returns an error for Hypercert access when token ID is missing", () => {
365+
const error = getAccessConfigValidationError(
366+
makeValues({
367+
accessType: AccessType.Hypercert,
368+
protocolConfig: {
369+
accessType: "Hypercert",
370+
hypercertAddress: "0x3333333333333333333333333333333333333333",
371+
},
372+
}),
373+
);
374+
expect(error).toBe("Hypercert token ID is required");
375+
});
376+
377+
it("returns an error for Hypercert access when token ID is non-numeric", () => {
378+
const error = getAccessConfigValidationError(
379+
makeValues({
380+
accessType: AccessType.Hypercert,
381+
protocolConfig: {
382+
accessType: "Hypercert",
383+
hypercertAddress: "0x3333333333333333333333333333333333333333",
384+
hypercertTokenId: "abc",
385+
},
386+
}),
387+
);
388+
expect(error).toBe("Hypercert token ID must be a valid number");
389+
});
390+
391+
it("returns an error for Hypercert access when min balance is invalid", () => {
392+
const error = getAccessConfigValidationError(
393+
makeValues({
394+
accessType: AccessType.Hypercert,
395+
protocolConfig: {
396+
accessType: "Hypercert",
397+
hypercertAddress: "0x3333333333333333333333333333333333333333",
398+
hypercertTokenId: "1",
399+
hypercertMinBalance: Number.NaN,
400+
},
401+
}),
402+
);
403+
expect(error).toBe("Hypercert minimum balance must be a valid number");
404+
});
405+
406+
it("returns undefined for valid Hypercert access", () => {
407+
const error = getAccessConfigValidationError(
408+
makeValues({
409+
accessType: AccessType.Hypercert,
410+
protocolConfig: {
411+
accessType: "Hypercert",
412+
hypercertAddress: "0x3333333333333333333333333333333333333333",
413+
hypercertTokenId: "42",
414+
hypercertMinBalance: 1,
415+
},
416+
}),
417+
);
418+
expect(error).toBeUndefined();
419+
});
420+
421+
it("returns an error for Hats access when hatsId is missing", () => {
422+
const error = getAccessConfigValidationError(
423+
makeValues({
424+
accessType: AccessType.Hats,
425+
protocolConfig: { accessType: "Hats" },
426+
}),
427+
);
428+
expect(error).toBe("Hat ID is required");
429+
});
430+
431+
it("returns an error for Hats access when hatsId is non-numeric", () => {
432+
const error = getAccessConfigValidationError(
433+
makeValues({
434+
accessType: AccessType.Hats,
435+
protocolConfig: { accessType: "Hats", hatsId: "abc" },
436+
}),
437+
);
438+
expect(error).toBe("Hat ID must be a valid number");
439+
});
440+
441+
it("returns undefined for valid Hats access", () => {
442+
const error = getAccessConfigValidationError(
443+
makeValues({
444+
accessType: AccessType.Hats,
445+
protocolConfig: { accessType: "Hats", hatsId: "99" },
446+
}),
447+
);
448+
expect(error).toBeUndefined();
212449
});
213450
});

‎client/components/create/StepContent.tsx‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -606,9 +606,24 @@ const AccessControlStep: React.FC = () => {
606606

607607
const handleAddNFT = useCallback(
608608
(address: string, type: number) => {
609+
const normalizedAddress = address.trim().toLowerCase();
610+
if (!normalizedAddress) return;
611+
609612
const currentAddresses = getValues("nftAddresses");
610613
const currentTypes = getValues("nftTypes");
611-
setValue("nftAddresses", [...currentAddresses, address]);
614+
const normalizedCurrentAddresses = currentAddresses.map((item) =>
615+
item.trim().toLowerCase(),
616+
);
617+
const existingIndex = normalizedCurrentAddresses.indexOf(normalizedAddress);
618+
619+
if (existingIndex !== -1) {
620+
const nextTypes = [...currentTypes];
621+
nextTypes[existingIndex] = type as NFTType;
622+
setValue("nftTypes", nextTypes);
623+
return;
624+
}
625+
626+
setValue("nftAddresses", [...currentAddresses, normalizedAddress]);
612627
setValue("nftTypes", [...currentTypes, type as NFTType]);
613628
},
614629
[getValues, setValue],

0 commit comments

Comments
 (0)