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 FXIOS-10362 - Enable address autofill edit by default on US and CA #23256

Conversation

issammani
Copy link
Collaborator

@issammani issammani commented Nov 20, 2024

📜 Tickets

Jira ticket
Github issue

💡 Description

This PR:

  • Enables address edit by default on US and CA or if user is enrolled in an experiment that enables it.

⚠️ NOTE1: I am intentionally not removing the nimbus flag because we intend to rollout this feature to other locales soon.
⚠️ NOTE2: This needs to be backported to 133.

📝 Checklist

You have to check all boxes before merging

  • Filled in the above information (tickets numbers and description of your work)
  • Updated the PR name to follow our PR naming guidelines
  • Wrote unit tests and/or ensured the tests suite is passing
  • When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
  • If needed, I updated documentation / comments for complex code and public methods
  • If needed, added a backport comment (example @Mergifyio backport release/v120)

@issammani issammani requested a review from nbhasin2 November 20, 2024 13:29
@issammani issammani requested a review from a team as a code owner November 20, 2024 13:29
@mobiletest-ci-bot
Copy link

mobiletest-ci-bot commented Nov 20, 2024

Messages
📖 Project coverage: 32.85%
📖 Edited 1 files
📖 Created 0 files

Client.app: Coverage: 31.54

File Coverage
AddressListViewModel.swift 46.27% ⚠️

Generated by 🚫 Danger Swift against fe49ec8

@issammani issammani changed the title FXIOS-10362 - Enable address autofill edit by default on US and CA Add FXIOS-10362 - Enable address autofill edit by default on US and CA Nov 20, 2024
Comment on lines +38 to +41
var isEditingFeatureEnabled: Bool {
AddressLocaleFeatureValidator.isValidRegion() ||
featureFlags.isFeatureEnabled(.addressAutofillEdit, checking: .buildOnly)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@issammani won't this enable for all even if the reason is valid?

I am not sure if we want the logic to first check feature enabled then validate via region

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unless I am missing something featureFlags.isFeatureEnabled(.addressAutofillEdit, checking: .buildOnly) should only be true if the user is enrolled in the experiment ? My reasoning to leave it there was because we want to rollout to germany and france at some point so we will need a nimbus flag again. But I can remove it from this check and keep the flag in the nimbus manifest for later use. What do you think ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah sorry, I understand why we need the || with your original change. Please change it back to the way it was before :)

@issammani issammani requested a review from nbhasin2 November 22, 2024 17:13
@issammani issammani force-pushed the chore/enabled-address-edit-by-default-for-us-ca branch from 18ba9b8 to 8f82957 Compare November 22, 2024 17:28
@issammani issammani force-pushed the chore/enabled-address-edit-by-default-for-us-ca branch from 8f82957 to fe49ec8 Compare November 22, 2024 19:55
@issammani
Copy link
Collaborator Author

@Mergifyio backport release/v133

Copy link
Contributor

mergify bot commented Nov 22, 2024

backport release/v133

✅ Backports have been created

@issammani issammani merged commit 6e1f58c into mozilla-mobile:main Nov 22, 2024
6 checks passed
mergify bot pushed a commit that referenced this pull request Nov 22, 2024
#23256)

chore: enable address edit by default on US and CA
(cherry picked from commit 6e1f58c)
clarmso pushed a commit that referenced this pull request Nov 25, 2024
#23256)

chore: enable address edit by default on US and CA
isabelrios pushed a commit that referenced this pull request Dec 3, 2024
#23256)

chore: enable address edit by default on US and CA
DonalMe pushed a commit that referenced this pull request Dec 3, 2024
…A (backport #23256) (#23337)

Add FXIOS-10362 - Enable address autofill edit by default on US and CA (#23256)

chore: enable address edit by default on US and CA
(cherry picked from commit 6e1f58c)

Co-authored-by: Issam Mani <[email protected]>
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