Allow setting name of FactorSources

[matiasbzurovski]

Exports a UniFFI function that allows setting name of any FactorSource.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.3%. Comparing base (7f13683) to head (a157afc).
Report is 1 commits behind head on main.

Additional details and impacted files

@@          Coverage Diff          @@
##            main    #305   +/-   ##
=====================================
  Coverage   93.3%   93.3%           
=====================================
  Files       1109    1109           
  Lines      23528   23547   +19     
  Branches      79      79           
=====================================
+ Hits       21955   21974   +19     
  Misses      1558    1558           
  Partials      15      15           

Flag	Coverage Δ
kotlin	97.1% <ø> (ø)
rust	92.8% <100.0%> (+<0.1%)	⬆️
swift	94.8% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...s_card_factor_source/arculus_card_factor_source.rs	100.0% <100.0%> (ø)
...factor_source/off_device_mnemonic_factor_source.rs	100.0% <100.0%> (ø)
...s/password_factor_source/password_factor_source.rs	100.0% <100.0%> (ø)
..._factor_source/security_questions_factor_source.rs	100.0% <ø> (ø)
...act_factor_source/trusted_contact_factor_source.rs	100.0% <100.0%> (ø)
...s/sargon/src/profile/v100/factors/factor_source.rs	99.1% <100.0%> (+<0.1%)	⬆️
...urces/device_factor_source/device_factor_source.rs	90.0% <100.0%> (+0.3%)	⬆️
...tor_source/ledger_hardware_wallet_factor_source.rs	100.0% <100.0%> (ø)
...argon/src/profile/v100/factors/is_factor_source.rs	50.0% <ø> (ø)
...s/sargon/src/system/sargon_os/sargon_os_factors.rs	71.1% <100.0%> (+0.7%)	⬆️

[Sajjon]

minor improvement would be to change String -> impl AsRef<str> making it a bit easier to write unit tests, allowing you to pass string literals (&str) or String :)

[Sajjon]

and inside this function you do updated.as_ref().to_owned()

[matiasbzurovski]

I did remember this suggestion from previous PR review 🙂

I decided not to include it in this case since tests are very simple anyway, and in the end we add one .as_ref().to_owned() in production code, for each .to_owned() removed in tests (being the relation 1-1)

[Sajjon]

it is unfortunate that Rust is not as great as Swift when it comes to method with same name but different types - in Swift we could have written such a method on the protocol - the trait - and allow str/String. here it would be a collision.

[Sajjon]

LGTM - will approve on work phone tomorrow. anyone on work phone now can approve it for me :)

[GhenadieVP]

This is nice, but I think we can go a step further and also make the update in the Profile itself, so maybe this API should have the factor source id to be updated.

Or your intention is different?

[matiasbzurovski]

I was thinking of handling it the same way we handle Account name updates, but guess we can directly do altogether. Will update

[GhenadieVP]

That is more legacy :), we should migrate how we rename accounts sometime to be handled in Sargon.

[matiasbzurovski]

actually I am not sure if it is better to have a function that updates the name: String of a given factor_source_id: FactorSourceId. Considering Hosts we will already have access to the actual FactorSource and that we already have a function that updates a FactorSource, seems pointless to send the id just to have Sargon fetch it from the Profile.

Maybe instead the function should be like this

pub async fn update_factor_source_name(&self, factor_source: FactorSource, name: String) -> Result<()>

[GhenadieVP]

Sure that also works, the main idea is to have the Profile mutation in Sargon. You might want to return the update FactorSource as response of this function.