Skip to content
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

Migrate from SharedPreferences to DataStore #7355

Merged
merged 3 commits into from
Dec 19, 2024

Conversation

kl
Copy link
Contributor

@kl kl commented Dec 17, 2024


This change is Reviewable

Copy link

linear bot commented Dec 17, 2024

@kl kl added the Android Issues related to Android label Dec 17, 2024
@kl kl requested review from Rawa, albin-mullvad and Pururun December 17, 2024 15:45
@kl kl force-pushed the migrate-from-sharedpreferences-to-datastore-droid-1688 branch from 9f1f9fc to 2236645 Compare December 17, 2024 15:56
Pururun
Pururun previously approved these changes Dec 17, 2024
Copy link
Contributor

@Pururun Pururun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 13 of 13 files at r1.
Reviewable status: 12 of 13 files reviewed, all discussions resolved

@kl kl force-pushed the migrate-from-sharedpreferences-to-datastore-droid-1688 branch from 2236645 to 9b81d26 Compare December 17, 2024 15:58
Copy link
Contributor

@Rawa Rawa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 12 of 13 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @kl)


android/app/src/main/proto/user_prefs.proto line 8 at r2 (raw file):

message UserPreferences {
  bool is_privacy_disclosure_accepted = 1;
  bool is_latest_changelog_seen = 2;

Should be removed from this PR

@kl kl force-pushed the migrate-from-sharedpreferences-to-datastore-droid-1688 branch from 9b81d26 to 8c34ed0 Compare December 18, 2024 07:42
Copy link
Contributor Author

@kl kl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 12 of 13 files reviewed, 1 unresolved discussion (waiting on @Rawa)


android/app/src/main/proto/user_prefs.proto line 8 at r2 (raw file):

Previously, Rawa (David Göransson) wrote…

Should be removed from this PR

Fixed

Copy link
Contributor

@Rawa Rawa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @kl)


android/app/src/main/kotlin/net/mullvad/mullvadvpn/repository/UserPreferencesMigration.kt line 15 at r3 (raw file):

    fun migrations(context: Context): List<DataMigration<UserPreferences>> =
        listOf(
            SharedPreferencesMigration(context, sharedPreferencesName = APP_PREFERENCES_NAME) {

@albin-mullvad We don't have anything here historically that we would care about I presume?

Copy link
Contributor Author

@kl kl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @albin-mullvad)


android/app/src/main/kotlin/net/mullvad/mullvadvpn/repository/UserPreferencesMigration.kt line 15 at r3 (raw file):

Previously, Rawa (David Göransson) wrote…

@albin-mullvad We don't have anything here historically that we would care about I presume?

We do have the Split Tunneling migration but it looks like that is using a different shared preferences file called "split_tunneling". The file that contains the privacy disclosure key is called "${BuildConfig.APPLICATION_ID}.app_preferences"

So the question is, do we only need to care about the privacy disclosure key in "${BuildConfig.APPLICATION_ID}.app_preferences" or could other historical keys be in that file as well?

Copy link
Contributor

@Rawa Rawa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @kl)


android/app/src/main/kotlin/net/mullvad/mullvadvpn/repository/UserPreferencesMigration.kt line 30 at r4 (raw file):

                    .build()
            }
        )

🌟 Nice

Copy link
Contributor

@Rawa Rawa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@Rawa Rawa force-pushed the migrate-from-sharedpreferences-to-datastore-droid-1688 branch 4 times, most recently from ed5e2c9 to de924a3 Compare December 19, 2024 12:27
@Rawa Rawa force-pushed the migrate-from-sharedpreferences-to-datastore-droid-1688 branch from de924a3 to 3c259a8 Compare December 19, 2024 12:30
@Rawa Rawa merged commit ccd7525 into main Dec 19, 2024
37 checks passed
@Rawa Rawa deleted the migrate-from-sharedpreferences-to-datastore-droid-1688 branch December 19, 2024 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Android Issues related to Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants