Update the queries in user-fetchers to fetch upload_id and upload_secret from uploads table to create upload URL and include in ClientAvatar.
Details
Details
Set breakpoints after each query and observed the result set and that the ClientAvatars were constructed correctly.
Verified that I was able to see image avatars on native client:
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- arcpatch-D7385 (branched from master)
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
This was originally split up in a much cleaner way.
- Introduce convertDBAvatarToClientAvatar utility and corresponding unit tests
- Update user-fetchers queries to fetch avatar uploads and construct ClientImageAvatar with convertDBAvatarToClientAvatar
I generally prefer the pattern of breaking down my work, writing whatever self-contained abstractions/functions/utilities are necessary, and then composing them together... but whatever 🤷♂️
| keyserver/src/fetchers/user-fetchers.js | ||
|---|---|---|
| 36 ↗ | (On Diff #25007) | Can you remove the trailing space at the end of this line here? |
| 40 ↗ | (On Diff #25007) | Is there a risk of this returning multiple rows? I guess when we set a new avatar, we make sure to "orphan" any previous image avatars? Looking at it more closely, it looks like none of the code will "crash" if we have multiple uploads, but the behavior is pretty undefined in those cases. fetchUserInfos and fetchKnownUserInfos will return the avatar from the last row that matches, whereas fetchLoggedInUserInfo will return the avatar from the first row that matches |
| 60 ↗ | (On Diff #25007) | row.upload_secret should already be a string... no need to call toString() |
| 104 ↗ | (On Diff #25007) | Remove trailing space |
| 158 ↗ | (On Diff #25007) | row.upload_secret should already be a string... no need to call toString() |
| 320–330 ↗ | (On Diff #25007) | This code is repeated in 3 places. Should we introduce a utility function that takes dbAvatar, uploadID, and uploadSecret, and returns clientAvatar? (Can be done in a later diff if you prefer) |
| 325 ↗ | (On Diff #25007) | row.upload_secret should already be a string... no need to call toString() |
| keyserver/src/fetchers/user-fetchers.js | ||
|---|---|---|
| 40 ↗ | (On Diff #25007) |
A. We make sure that we set the avatar and unset previous avatars in a single transaction to make that it's atomic. |
| keyserver/src/fetchers/user-fetchers.js | ||
|---|---|---|
| 40 ↗ | (On Diff #25007) |
I think people are trying to move away from the use of the word "orphan" in this context, "disconnected" may be a suitable alternative. |