Details
Run tests to make sure existing logic is the same. For farcaster_tokens I connected my DataGrip to my localstack, added some logging in tests and for each tests I was checking if values are showing up or are deleted.
Diff Detail
- Repository
- rCOMM Comm
- Branch
- farcaster-identity-changes
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| services/identity/src/database/farcaster.rs | ||
|---|---|---|
| 71–75 | In theory, I should've updated this, too, but this will be a complicated operation where I don't think this is needed at all. I tried to implement a test for it as part D15206, and that case (need to remove DCs token) isn't reachable. Updating ID for a user who already has it is gated here, and it isn't possible to add a token when there is no ID here, I think we should just remove REMOVE {1} cc. (@tomek) | |
| services/identity/src/database/farcaster.rs | ||
|---|---|---|
| 71–75 | Yeah, I think it makes sense. Connecting a Farcaster account requires disconnecting first, which removes the token, so there are no valid scenarios where the token is set in this function. | |
| services/identity/src/database.rs | ||
|---|---|---|
| 318–323 | Let's avoid cloning when it's none | |
| services/identity/src/database/farcaster.rs | ||
| 139 | Separately, in the future this MissingItem could be improved to provide more details to LinkFarcasterDCsAccount RPC handler, but for now such log should be sufficient to investigate what went wrong | |
| 238 | You don't have to clone here | |
| 251–254 | A better way when transact items are fixed | |
| 255–263 | Do you think we should add exponential backoff to this transaction? Is there any chance Tunnelbroker tries to write it at the same time? | |
| 296 | DynamoDB errors better implement Debug trait than Display | |
- avoid clonning None
- delete REMOVE {1} clause
- improve logging
- add exponential backoff