[identity] Remove deprecated conversion traits
ClosedPublic
Actions

Authored by bartek on Jan 22 2024, 3:02 AM.

Details

Reviewers

michal
varun

Commits

rCOMM6232b76cb106: [identity] Remove deprecated conversion traits

Summary

After legacy device list code was removed, the conversion traits were no longer used. This removes them and replaces last usages with the proper calls from comm-lib. This will also simplify the social proof fix (next diffs).

I had to add implementation for optional attributes to comm-lib, since it was missing.

Depends on D10701

Test Plan

Identity integration tests
Unit tests for comm-lib to test optional attributes

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bartek created this revision.Jan 22 2024, 3:02 AM

bartek held this revision as a draft.

Herald added subscribers: tomek, ashoat. · View Herald TranscriptJan 22 2024, 3:02 AM

bartek added a child revision: D10764: [identity] Store social proof in users table.Jan 22 2024, 3:06 AM

Harbormaster completed remote builds in B25985: Diff 35917.Jan 22 2024, 3:41 AM

bartek published this revision for review.Jan 22 2024, 3:44 AM

bartek added inline comments.

services/identity/src/ddb_utils.rs
76 ↗	(On Diff #35917)	Clippy fix, same as D10257
shared/comm-lib/src/database.rs
173–180 ↗	(On Diff #35917)	Wondering if this should be simplified this way. Currently, all implementations of `TryFromAttribute` return `DBItemAttributeError::Missing` if the attribute is `None` and we're already checking that above. Can we think of other situations where `DBItemAttributeError::Missing` can be possibly returned?

varun accepted this revision.Jan 25 2024, 8:55 PM

varun added inline comments.

shared/comm-lib/src/database.rs
173–180 ↗	(On Diff #35917)	i can't think of another scenario where the `Missing` variant could be returned, but i still prefer the current syntax, even if it's more verbose