Page MenuHomePhabricator

[protos][identity] Add reserved usernames to UserIdentitiesResponse
ClosedPublic

Authored by bartek on Jul 29 2024, 11:32 AM.
Tags
None
Referenced Files
F2766242: D12922.diff
Thu, Sep 19, 3:26 PM
Unknown Object (File)
Tue, Sep 10, 4:15 PM
Unknown Object (File)
Sat, Sep 7, 1:43 PM
Unknown Object (File)
Sat, Sep 7, 1:43 PM
Unknown Object (File)
Sun, Sep 1, 4:45 AM
Unknown Object (File)
Sun, Sep 1, 4:44 AM
Unknown Object (File)
Sun, Sep 1, 4:43 AM
Unknown Object (File)
Sat, Aug 31, 7:54 PM
Subscribers

Details

Summary

First part of ENG-8771 - added a new response field as proposed in this comment.
This change is backwards compatible, regardless if we release identity first or clients first. The new field will simply default to an empty map.

Test Plan

Flow, cargo build. Logic is added in the next diffs

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek held this revision as a draft.
bartek published this revision for review.Jul 29 2024, 12:01 PM
bartek added inline comments.
services/identity/src/grpc_services/authenticated.rs
882 ↗(On Diff #42910)

Actual implementation in D12924

web/protobufs/identity-auth-structs.cjs.flow
621 ↗(On Diff #42910)

I noticed that everywhere else we use ProtoMap for types being defined as map<string, Whatever> in protos

Haven't reviewed the Rust but the changes seem minimal / safe

shared/protos/identity_auth.proto
323 ↗(On Diff #42910)

How about reserved_user_identifiers? I think it's weird to use the same noun ("identities") here when the type is different from identities above

web/protobufs/identity-auth-structs.cjs.flow
621 ↗(On Diff #42910)

Good find

This revision is now accepted and ready to land.Jul 30 2024, 10:51 PM
shared/protos/identity_auth.proto
323 ↗(On Diff #42910)

Yeah, it's better, thanks!
I'll update the names.

Rename to reserved_user_identifiers