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

Fix daita and multihop warning being shown when multihop is disabled #7242

Merged

Conversation

olmoh
Copy link
Collaborator

@olmoh olmoh commented Nov 26, 2024

Since the directOnly boolean is separate from the multihop setting we need to check the multihop setting as well before disabling the entry selection.


This change is Reviewable

@olmoh olmoh requested a review from raksooo November 26, 2024 15:22
Copy link
Contributor

@hulthe hulthe left a comment

Choose a reason for hiding this comment

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

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


desktop/packages/mullvad-vpn/src/renderer/components/select-location/SelectLocation.tsx line 313 at r1 (raw file):

        <CustomLists selectedElementRef={selectedLocationRef} onSelect={onSelectEntryRelay} />
        <LocationList
          key={locationType}

If we end up in this branch, locationType will be set to Entry. I think that would mean the LocationList shown here will have the wrong list of relays... We probably want to ensure that locationType is always set to exit when multihop is disabled.

Copy link
Contributor

@hulthe hulthe 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: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @olmoh)


desktop/packages/mullvad-vpn/src/renderer/components/select-location/SelectLocation.tsx line 313 at r1 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

If we end up in this branch, locationType will be set to Entry. I think that would mean the LocationList shown here will have the wrong list of relays... We probably want to ensure that locationType is always set to exit when multihop is disabled.

Hmm. this appears to be a bug that's been around longer than the daita-warning-bug. It's present in 2024.7.

To reproduce:

  • Enable multihop
  • Click "entry" in location view
  • Disable multihop
  • Select a new location

@olmoh olmoh force-pushed the daita-multihop-warning-is-shown-in-select-location-view-des-1506 branch from d1aaf8a to 51ae47c Compare November 27, 2024 14:26
Copy link
Contributor

@hulthe hulthe 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 1 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @olmoh)

hulthe
hulthe previously approved these changes Nov 27, 2024
Copy link
Contributor

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

@hulthe hulthe force-pushed the daita-multihop-warning-is-shown-in-select-location-view-des-1506 branch from e7203d4 to 192df00 Compare November 27, 2024 15:58
@hulthe hulthe force-pushed the daita-multihop-warning-is-shown-in-select-location-view-des-1506 branch from 192df00 to e9c6f50 Compare November 27, 2024 15:59
Copy link
Contributor

@hulthe hulthe 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 r6, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@hulthe hulthe requested a review from Serock3 November 27, 2024 16:09
Copy link
Contributor

@Serock3 Serock3 left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@hulthe hulthe merged commit 7ecd38c into main Nov 27, 2024
14 checks passed
@hulthe hulthe deleted the daita-multihop-warning-is-shown-in-select-location-view-des-1506 branch November 27, 2024 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants