Paths

Table of Contentst

Differential D11411

[identity] don't let linkFarcasterAccount overwrite existing fid for user
ClosedPublic
Actions

Authored by varun on Mar 27 2024, 8:26 PM.

Tags

None

Referenced Files

	F32509292: D11411.1767101390.diff
	Mon, Dec 29, 1:29 PM

Subscribers

Details

Reviewers

Commits

rCOMM8ec3ffdc4d77: [identity] don't let linkFarcasterAccount overwrite existing fid for user

Summary

https://linear.app/comm/issue/ENG-7358/make-sure-only-1-fid-is-associated-with-a-given-comm-user

Depends on D11410

Test Plan

tried calling linkFarcasterAccount multiple times. attempts with the same FID succeeded but attempts with a different FID failed.

Diff Detail

Repository

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

varun created this revision.Mar 27 2024, 8:26 PM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptMar 27 2024, 8:26 PM

varun added a child revision: D11412: [identity] check if fid is already taken when registering new user.Mar 27 2024, 8:28 PM

Harbormaster completed remote builds in B27726: Diff 38389.Mar 27 2024, 8:40 PM

varun requested review of this revision.Mar 27 2024, 8:40 PM

ashoat added a comment.Mar 28 2024, 12:28 AM

This comment was removed by ashoat.

bartek accepted this revision.Mar 28 2024, 6:20 AM

bartek added inline comments.

services/identity/src/database/farcaster.rs
68–72	You don't have to provide the same value twice
88	Does `UpdateExpression` `if_not_exists` fail the operation if given value already exists? The docs don't say this explicitly, but I guess `if_not_exists (path, value)` If the item does not contain an attribute at the specified `path`, `if_not_exists` evaluates to `value`; otherwise, it evaluates to `path`. means that if the ID already exists, the expression evaluates to `SET farcasterID = farcasterID` so it's a no-op. According to your test plan, it succeeds so it's alright

This revision is now accepted and ready to land.Mar 28 2024, 6:20 AM

varun added inline comments.Mar 28 2024, 9:33 PM

services/identity/src/database/farcaster.rs
88	yeah i checked and it succeeds so assuming DDB treats it as a no-op

Closed by commit rCOMM8ec3ffdc4d77: [identity] don't let linkFarcasterAccount overwrite existing fid for user. · Explain WhyMar 28 2024, 10:17 PM

This revision was automatically updated to reflect the committed changes.

varun added a commit: rCOMM8ec3ffdc4d77: [identity] don't let linkFarcasterAccount overwrite existing fid for user.

Revision Contents
Changeset List

Path

Size

services/

identity/

src/

database/

28 lines

2 lines

Diff 38389

services/identity/src/database/farcaster.rs

Loading...

services/identity/src/error.rs

Loading...