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

Add UI for DAITA in VPN settings #6667

Merged
merged 1 commit into from
Aug 28, 2024
Merged

Conversation

mojganii
Copy link
Collaborator

@mojganii mojganii commented Aug 23, 2024

This PR introduces the ability to toggle Daita in settings:

before after

This change is Reviewable

@mojganii mojganii added the iOS Issues related to iOS label Aug 23, 2024
@mojganii mojganii requested a review from rablador August 23, 2024 10:42
@mojganii mojganii self-assigned this Aug 23, 2024
Copy link

linear bot commented Aug 23, 2024

Copy link
Contributor

@rablador rablador left a comment

Choose a reason for hiding this comment

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

Reviewed 16 of 16 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @mojganii)


ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsDataSource.swift line 643 at r1 (raw file):

            } onDiscard: { [weak self] in
                guard let self else { return }
                tableView?.reloadData()

Why do we need to manually reload the tableView here?


ios/PacketTunnelCoreTests/PacketTunnelActorTests.swift line 449 at r1 (raw file):

        await fulfillment(of: [stopMonitorExpectation], timeout: .UnitTest.timeout)
    }

Leftover?

Copy link
Contributor

@buggmagnet buggmagnet left a comment

Choose a reason for hiding this comment

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

Reviewed 15 of 16 files at r1.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @mojganii)


ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsViewController.swift line 192 at r1 (raw file):

            tableName: "DAITA",
            value: """
            DAITA (Defence against AI-guided Traffic Analysis) hides patterns in your encrypted VPN traffic.\

Screenshot 2024-08-27 at 12.00.59.png
The text needs some spaces here and there.


ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsViewController.swift line 235 at r1 (raw file):

                    tableName: "DAITA",
                    value: """
                    This feature isn't available on all servers.\

Screenshot 2024-08-27 at 12.03.58.png
Same here, the text needs some extra spaces


ios/PacketTunnelCore/Actor/PacketTunnelActor+ErrorState.swift line 107 at r1 (raw file):

            keyPolicy: connState.keyPolicy,
            networkReachability: connState.networkReachability,
            recoveryTask: startRecoveryTaskIfNeeded(reason: reason),

Why are we adding this here ?

Copy link
Collaborator Author

@mojganii mojganii 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, 5 unresolved discussions (waiting on @buggmagnet and @rablador)


ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsDataSource.swift line 643 at r1 (raw file):

Previously, rablador (Jon Petersson) wrote…

Why do we need to manually reload the tableView here?

To turn the toggle off if user taps back button in the dialog.


ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsViewController.swift line 192 at r1 (raw file):

Previously, buggmagnet wrote…

Screenshot 2024-08-27 at 12.00.59.png
The text needs some spaces here and there.

Done.


ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsViewController.swift line 235 at r1 (raw file):

Previously, buggmagnet wrote…

Screenshot 2024-08-27 at 12.03.58.png
Same here, the text needs some extra spaces

Done.


ios/PacketTunnelCore/Actor/PacketTunnelActor+ErrorState.swift line 107 at r1 (raw file):

Previously, buggmagnet wrote…

Why are we adding this here ?

the branch must be rebased with main. now it's updated and resolved


ios/PacketTunnelCoreTests/PacketTunnelActorTests.swift line 449 at r1 (raw file):

Previously, rablador (Jon Petersson) wrote…

Leftover?

Solved.

@mojganii mojganii force-pushed the add-daita-ui-part-ios-794 branch 2 times, most recently from d51f0c3 to 95e8f1d Compare August 28, 2024 09:56
Copy link
Contributor

@buggmagnet buggmagnet left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @rablador)

Copy link
Contributor

@buggmagnet buggmagnet left a comment

Choose a reason for hiding this comment

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

Dismissing @rablador so we can merge the PR as all his claims have been adressed

Dismissed @rablador from 2 discussions.
Reviewable status: all files reviewed, 2 unresolved discussions

@buggmagnet buggmagnet dismissed rablador’s stale review August 28, 2024 12:25

Reviewer is satisfied, just unavailable to say they are

@buggmagnet buggmagnet force-pushed the add-daita-ui-part-ios-794 branch from 95e8f1d to cbce176 Compare August 28, 2024 12:28
@buggmagnet buggmagnet merged commit 1a1988b into main Aug 28, 2024
8 of 9 checks passed
@buggmagnet buggmagnet deleted the add-daita-ui-part-ios-794 branch August 28, 2024 12:30
Copy link

🚨 End to end tests failed. Please check the failed workflow run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iOS Issues related to iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants