-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(backend): methods to save the networks list settings #5355
feat(backend): methods to save the networks list settings #5355
Conversation
8a9133c
to
9709f5a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces new backend methods and endpoints to save a user's networks list settings by either overwriting or merging with existing settings. It also updates shared types and DID declarations to support these new methods while maintaining existing testnets settings functionality.
- Added the with_networks function in the shared impls to handle both overwriting and merging logic.
- Introduced set_network_settings and update_network_settings functions in the user_profile module.
- Added new endpoints in lib.rs and updated shared types and DID declarations to support network settings updates.
Reviewed Changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/backend/tests/it/settings/testnets_settings.rs | Tests for testnets settings remain and ensure backwards compatibility. |
src/shared/src/impls.rs | Adds the with_networks method to update networks settings based on an overwrite flag. |
src/backend/src/user_profile.rs | Introduces set_network_settings and update_network_settings functions using with_networks. |
src/backend/src/lib.rs | Adds endpoints for set_user_network_settings and update_user_network_settings. |
src/shared/src/types.rs | Updates type definitions by aliasing networks map and adding error types and request types for network settings. |
src/declarations/backend/backend.factory.did.js | Updates DID declarations to include the new network settings endpoints and request types. |
src/declarations/backend/backend.factory.certified.did.js | Mirrors the changes in DID declarations for certified calls. |
Files not reviewed (2)
- src/backend/backend.did: Language not supported
- src/declarations/backend/backend.did: Language not supported
Comments suppressed due to low confidence (2)
src/backend/tests/it/settings/testnets_settings.rs:1
- Consider adding tests specifically for the new network settings endpoints (set_network_settings and update_network_settings) to ensure full coverage of the new functionality.
use candid::Principal;
src/shared/src/impls.rs:189
- Verify that the version check 'if profile_version != self.version' handles the case when profile_version is None as intended, ensuring no unintended behavior in version management.
pub fn with_networks(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. LGTM!
Motivation
We will start to save the enabled/disabled networks in the user's profile settings.
So, we provide two methods in the backend to do that: one to update existing networks settings and the other to overwrite them.
Changes
with_networks
that overwrite or updates (depending on the input flag) the user's profile networks settings.set_network_settings
that only overwrite the networks settings.update_network_settings
that only merges the input values with the existing networks settings.Tests
Provided new tests for the new endpoints.
UNRELATED: I split the tests for
testnets
settings from the ones fromnetworks
settings for better manageability.