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

List only DAITA enabled entry locations in locations view #6609

Conversation

rablador
Copy link
Contributor

@rablador rablador commented Aug 13, 2024

When DAITA is enabled, the app should only list DAITA enabled entry locations in the locations view.

If the user has enabled both DAITA & multihop and the relay selector fails to match an entry, when the location view is opened, it should default to showing the entry location view.


This change is Reviewable

@rablador rablador changed the title List only daita enabled entry locations in locations view if ios 774 List only DAITA enabled entry locations in locations view Aug 13, 2024
@rablador rablador force-pushed the list-only-daita-enabled-entry-locations-in-locations-view-if-ios-774 branch 2 times, most recently from c0839d1 to 2272c98 Compare August 13, 2024 14:08
Copy link
Contributor Author

@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.

This PR also fixes the bug where closing a filter chip in either entry or exit (when multihop is on) doesn't close the other as well.

Reviewable status: 0 of 36 files reviewed, all discussions resolved

@rablador rablador force-pushed the list-only-daita-enabled-entry-locations-in-locations-view-if-ios-774 branch 5 times, most recently from 01c00b7 to 9076896 Compare August 15, 2024 12:14
@rablador rablador changed the base branch from main to allow-relay-selector-to-filter-daita-enabled-relays-ios-772 August 16, 2024 07:06
@rablador rablador force-pushed the list-only-daita-enabled-entry-locations-in-locations-view-if-ios-774 branch 2 times, most recently from e42edff to 69be154 Compare August 16, 2024 10:17
@rablador rablador force-pushed the allow-relay-selector-to-filter-daita-enabled-relays-ios-772 branch 3 times, most recently from 5cd5d21 to 5d337df Compare August 19, 2024 09:26
@buggmagnet buggmagnet force-pushed the allow-relay-selector-to-filter-daita-enabled-relays-ios-772 branch from 5d337df to b5baa95 Compare August 21, 2024 09:34
Base automatically changed from allow-relay-selector-to-filter-daita-enabled-relays-ios-772 to main August 21, 2024 09:37
@rablador rablador force-pushed the list-only-daita-enabled-entry-locations-in-locations-view-if-ios-774 branch 3 times, most recently from b9eb3c8 to 31e7c41 Compare August 21, 2024 13:41
@rablador rablador added the iOS Issues related to iOS label Aug 21, 2024
@rablador rablador self-assigned this Aug 21, 2024
@rablador rablador force-pushed the list-only-daita-enabled-entry-locations-in-locations-view-if-ios-774 branch from 31e7c41 to eabf71a Compare August 21, 2024 13:44
@rablador rablador marked this pull request as ready for review August 21, 2024 13:45
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 12 of 13 files at r1, all commit messages.
Reviewable status: 12 of 13 files reviewed, 1 unresolved discussion (waiting on @rablador)


ios/MullvadVPN/View controllers/SelectLocation/LocationViewController.swift line 29 at r1 (raw file):

    private var filter = RelayFilter()
    private var selectedRelays: RelaySelection
    private var daitaEnabled = false

Shouldn't that be passed by via Dependency Injection instead ? (Like it is for LocationViewControllerWrapper)

@rablador rablador force-pushed the list-only-daita-enabled-entry-locations-in-locations-view-if-ios-774 branch from eabf71a to aa0e997 Compare August 22, 2024 06:30
@rablador rablador force-pushed the list-only-daita-enabled-entry-locations-in-locations-view-if-ios-774 branch from aa0e997 to c0f8355 Compare August 22, 2024 07:48
Copy link
Contributor Author

@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.

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


ios/MullvadVPN/View controllers/SelectLocation/LocationViewController.swift line 29 at r1 (raw file):

Previously, buggmagnet wrote…

Shouldn't that be passed by via Dependency Injection instead ? (Like it is for LocationViewControllerWrapper)

Indeed!

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.

:lgtm:

Reviewed 1 of 13 files at r1, 2 of 2 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@buggmagnet buggmagnet force-pushed the list-only-daita-enabled-entry-locations-in-locations-view-if-ios-774 branch from c0f8355 to 847e43c Compare August 22, 2024 11:13
@buggmagnet buggmagnet merged commit ddf60c8 into main Aug 22, 2024
9 checks passed
@buggmagnet buggmagnet deleted the list-only-daita-enabled-entry-locations-in-locations-view-if-ios-774 branch August 22, 2024 11:35
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.

2 participants