-
Notifications
You must be signed in to change notification settings - Fork 9
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
[ABW-2214] Shared persona data not persisted in profile #734
Merged
CyonAlexRDX
merged 28 commits into
main
from
ABW-2214_shared_persona_data_not_persisted_in_profile
Sep 11, 2023
Merged
Changes from 17 commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
105c690
Align naming with Android
maciek-rdx 0cc3e11
Update RET
maciek-rdx 76fb812
Update Gateway
maciek-rdx 6fdbe71
Align names
maciek-rdx fd3eb27
Update the snapshot
maciek-rdx c1b8ac6
Update tests
maciek-rdx 37bb0d9
downgrade RET to 0.12.1-293dd4c which is used by GW and Android. Bump…
CyonAlexRDX ecdd8bc
WIP
CyonAlexRDX 8ffad72
WIP
CyonAlexRDX c9d18ec
WIP
CyonAlexRDX bd33190
WIP
CyonAlexRDX 2069f8c
Merge branch 'main' into ABW-2214_shared_persona_data_not_persisted_i…
CyonAlexRDX 0e04719
WIP
CyonAlexRDX d39b310
WIP
CyonAlexRDX 39a568c
Merge branch 'main' into ABW-2214_shared_persona_data_not_persisted_i…
CyonAlexRDX df6331c
fix bad merge
CyonAlexRDX 68e4f1f
revert profile test change
CyonAlexRDX 18fdb22
WIP
CyonAlexRDX 2d874a8
Merge branch 'main' into ABW-2214_shared_persona_data_not_persisted_i…
CyonAlexRDX f89fb8d
Delete references to shared entry if it gets deleted from persona
CyonAlexRDX 729876c
WIP
CyonAlexRDX 9d83bb5
WIP
CyonAlexRDX 1efa8d5
Make sure IDs of persona data entries are NOT changed when editing pe…
CyonAlexRDX 35bee56
WIP
CyonAlexRDX 1ee5db9
WIP
CyonAlexRDX ae8a69e
WIP
CyonAlexRDX 4b9b6e7
WIP
CyonAlexRDX ef07c89
WIP
CyonAlexRDX File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 changes: 1 addition & 1 deletion
2
Sources/Core/SharedModels/P2P/Dapp/Models/Interaction/Requests/ResetRequestItem.swift
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,6 +36,7 @@ public struct AuthorizedDapps: Sendable, FeatureReducer { | |
case loadedDapps(TaskResult<Profile.Network.AuthorizedDapps>) | ||
case loadedThumbnail(URL, dApp: Profile.Network.AuthorizedDapp.ID) | ||
case presentDappDetails(DappDetails.State) | ||
case failedToGetDetailsOfDapp(id: Profile.Network.AuthorizedDapp.ID) | ||
} | ||
|
||
public enum ChildAction: Sendable, Equatable { | ||
|
@@ -56,17 +57,16 @@ public struct AuthorizedDapps: Sendable, FeatureReducer { | |
public func reduce(into state: inout State, viewAction: ViewAction) -> EffectTask<Action> { | ||
switch viewAction { | ||
case .appeared: | ||
return .task { | ||
await loadAuthorizedDapps() | ||
} | ||
return loadAuthorizedDapps() | ||
|
||
case let .didSelectDapp(dAppID): | ||
return .run { send in | ||
let details = try await authorizedDappsClient.getDetailedDapp(dAppID) | ||
let presentedDappState = DappDetails.State(dApp: details) | ||
await send(.internal(.presentDappDetails(presentedDappState))) | ||
} catch: { error, _ in | ||
} catch: { error, send in | ||
errorQueue.schedule(error) | ||
await send(.internal(.failedToGetDetailsOfDapp(id: dAppID))) | ||
} | ||
} | ||
} | ||
|
@@ -86,6 +86,19 @@ public struct AuthorizedDapps: Sendable, FeatureReducer { | |
} | ||
} | ||
} | ||
|
||
case let .failedToGetDetailsOfDapp(dappId): | ||
#if DEBUG | ||
return .run { _ in | ||
CyonAlexRDX marked this conversation as resolved.
Show resolved
Hide resolved
|
||
loggerGlobal.notice("DEBUG ONLY deleting authorized dapp since we failed to load detailed info about it") | ||
try? await authorizedDappsClient.forgetAuthorizedDapp(dappId, nil) | ||
|
||
}.concatenate(with: loadAuthorizedDapps()) | ||
#else | ||
// FIXME: Should we have to handle this, this is a discrepancy bug.. | ||
return .none | ||
#endif | ||
|
||
case let .loadedDapps(.failure(error)): | ||
errorQueue.schedule(error) | ||
return .none | ||
|
@@ -103,18 +116,19 @@ public struct AuthorizedDapps: Sendable, FeatureReducer { | |
case .presentedDapp(.presented(.delegate(.dAppForgotten))): | ||
return .run { send in | ||
await send(.child(.presentedDapp(.dismiss))) | ||
await send(loadAuthorizedDapps()) | ||
} | ||
}.concatenate(with: loadAuthorizedDapps()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. minor: move on next line. |
||
|
||
case .presentedDapp: | ||
return .none | ||
} | ||
} | ||
|
||
private func loadAuthorizedDapps() async -> Action { | ||
let result = await TaskResult { | ||
try await authorizedDappsClient.getAuthorizedDapps() | ||
private func loadAuthorizedDapps() -> EffectTask<Action> { | ||
CyonAlexRDX marked this conversation as resolved.
Show resolved
Hide resolved
|
||
.run { send in | ||
let result = await TaskResult { | ||
try await authorizedDappsClient.getAuthorizedDapps() | ||
} | ||
await send(.internal(.loadedDapps(result))) | ||
} | ||
return .internal(.loadedDapps(result)) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Since we're doing this in any case, what is the purpose of even allowing caller to submit a networkID? It's not performance, so is it the case that you might want to use some other networkID?
Either way, might as well do
And I would also rename
maybeNetworkID
toexplicitNetworkID
, if that is the purpose.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.
always safer to use explicit networkIDs, but I was in a context where we did not have it available.
And yes, ofc I tried that spelling :D but Xcode 14.3.1 is drunk, I get this compilation error, why I had to break it up :D