Page MenuHomePhabricator

[keyserver] fetchUsernames helper function
ClosedPublic

Authored by varun on Jun 27 2023, 7:29 AM.
Tags
None
Referenced Files
F3369572: D8330.id28170.diff
Mon, Nov 25, 10:55 PM
F3368421: D8330.diff
Mon, Nov 25, 7:47 PM
Unknown Object (File)
Tue, Nov 12, 6:08 PM
Unknown Object (File)
Sat, Nov 9, 6:59 AM
Unknown Object (File)
Thu, Oct 31, 1:32 PM
Unknown Object (File)
Oct 14 2024, 6:01 PM
Unknown Object (File)
Oct 11 2024, 1:28 PM
Unknown Object (File)
Oct 11 2024, 1:28 PM
Subscribers

Details

Summary

gets all the usernames from the users table

Test Plan

tested with subsequent diff

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

varun requested review of this revision.Jun 27 2023, 7:46 AM

Let's rename fetchUsernames to fetchAllUsernames before landing.

keyserver/src/fetchers/user-fetchers.js
377

Can we rename this to fetchAllUsernames so it's a little clearer + symmetric w/ fetchAllUserIDs above

379–383

In fetchAllUserIDs, we do:

const query = SQL`SELECT id FROM users`;
const [result] = await dbQuery(query);
return result.map(row => row.id.toString());

can we match that? Up to you

This revision is now accepted and ready to land.Jun 27 2023, 9:14 AM
keyserver/src/fetchers/user-fetchers.js
379–383

If the result is empty, I think your suggested code will throw an error because you can't call map on undefined. I'm guessing this is fine for id since it's the primary key, but I don't think it is for username

keyserver/src/fetchers/user-fetchers.js
379–383

@varun I don't follow your reasoning. If you're afraid that result can be null or undefined, then the same risk occurs on line 380, when you call result.length.

In fact, result should never be null or undefined... if dbQuery encounters an error then it will throw, and if it doesn't then result will be an array.

keyserver/src/fetchers/user-fetchers.js
379–383

ah fair. i'll change this

This revision was automatically updated to reflect the committed changes.