Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

Fix LocalData - isInteroperabilityShownAtLeastOnce - set(value) to commit=true #1451

Closed

Conversation

vaubaehn
Copy link
Contributor

Related: #1421 (comment) and #1421 (comment)

Description

While discussing persistence using LocalData with @kolyaopahle , we found a small flaw in LocalData

/****************************************************
* INTEROPERABILITY
****************************************************/
var isInteroperabilityShownAtLeastOnce: Boolean
get() {
return getSharedPreferenceInstance().getBoolean(
PREFERENCE_INTEROPERABILITY_IS_USED_AT_LEAST_ONCE,
false
)
}
set(value) {
getSharedPreferenceInstance().edit {
putBoolean(PREFERENCE_INTEROPERABILITY_IS_USED_AT_LEAST_ONCE, value)
}
}
}

For isInteroperabilityShownAtLeastOnce the getSharedPreferenceInstance().edit lacks the commit=true. Would be nice to fix it for consistency of the code.

@kolyaopahle kindly invited me to create a PR for this minor fix, but I must admit, I'd be proud like Oscar if it was to be merged.

Steps to reproduce

Investigate the code.

d4rken and others added 4 commits October 21, 2020 08:56
…EXPOSUREAPP-3313, EXPOSUREAPP-3335) (corona-warn-app#1433)

* I overlooked that `KeyException` is a subclass of `GeneralSecurityException`.
This causes us to continue with `KeyException` which then fails the message matching test.
As the underlying Tink code throws the GeneralSecurityException first, we check the type of the last exception.

* Add additional specific test cases for nested exceptions.

* The dialog shouldn't automatically dismiss when the user clicks details.
They may not have read the dialog message completely or want to re-read it.
corona-warn-app#1439)

* Increase transaction timeout to 8 minutes.

* Fix timeout variable test.

* Adjust code comment to mention the 10min limit.

Co-authored-by: Ralf Gehrer <ralfgehrer@users.noreply.github.com>
Co-authored-by: harambasicluka <64483219+harambasicluka@users.noreply.github.com>

Co-authored-by: Ralf Gehrer <ralfgehrer@users.noreply.github.com>
Co-authored-by: harambasicluka <64483219+harambasicluka@users.noreply.github.com>
@vaubaehn vaubaehn requested a review from a team October 21, 2020 16:34
@d4rken
Copy link
Member

d4rken commented Oct 21, 2020

Thanks for your PR. Can you target branch release/1.6.x?
Only PRs related to hotfixes of 1.5.x will still be merged into release/1.5.x.

@d4rken d4rken added the community Tag issues created by community members label Oct 21, 2020
@d4rken d4rken linked an issue Oct 21, 2020 that may be closed by this pull request
@d4rken d4rken added the bug Something isn't working label Oct 21, 2020
@vaubaehn vaubaehn changed the base branch from release/1.5.x to release/1.6.x October 21, 2020 17:19
@vaubaehn
Copy link
Contributor Author

vaubaehn commented Oct 21, 2020

@d4rken

Thanks for your PR. Can you target branch release/1.6.x?

solved?
edit: Obviously not.... Will try to rebase.

@d4rken
Copy link
Member

d4rken commented Oct 21, 2020

Not quite, now this PR will merge 1.5.x (including your change) into 1.6.x. (see the changed files tab now including the hotfixes). This needs to happen too, but not in this PR.

You need to rebase your branch onto 1.6.x and force push, or close the PR and make a new one with a new branch, depends a bit how comfortable you are with git.

@vaubaehn
Copy link
Contributor Author

You need to rebase your branch onto 1.6.x and force push, or close the PR and make a new one with a new branch, depends a bit how comfortable you are with git.

Lesson learned. Will close and reopen.
Sorry to keep you busy with this 😞

@vaubaehn vaubaehn closed this Oct 21, 2020
@vaubaehn vaubaehn deleted the release/1.5.x branch October 21, 2020 17:38
@d4rken
Copy link
Member

d4rken commented Oct 21, 2020

Sorry to keep you busy with this

I don't mind. The CWA is an open-source project and empowering users to contribute and participate is part of that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working community Tag issues created by community members
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Result "status" disappeared in the app after restarting it
3 participants