Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Issue #10205: Add support for select credit card prompt #10213

Merged
merged 1 commit into from
May 11, 2021

Conversation

gabrielluong
Copy link
Member

Fixes #10205

  • Implements an override function of onCreditCardSelect in GeckoPromptDelegate
  • Adds a new CreditCard data class in concept-engine. This is a parallel of GV's
    Autocomplete.CreditCard. We can't using the existing CreditCard from concept-storage
    since that has encryption dependencies whereas the card number is already decrypted
    when it reaches GV.
  • Adds a new SelectCreditCardPrompt in PromptRequest

Pull Request checklist

  • Quality: This PR builds and passes detekt/ktlint checks (A pre-push hook is recommended)
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry or does not need one
  • Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

After merge

  • Milestone: Make sure issues closed by this pull request are added to the milestone of the version currently in development.
  • Breaking Changes: If this is a breaking change, please push a draft PR on Reference Browser to address the breaking issues.

@gabrielluong gabrielluong added the 🕵️‍♀️ needs review PRs that need to be reviewed label May 4, 2021
@codecov
Copy link

codecov bot commented May 4, 2021

Codecov Report

Merging #10213 (bf725a2) into master (471da2b) will increase coverage by 2.35%.
The diff coverage is 97.67%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #10213      +/-   ##
============================================
+ Coverage     73.99%   76.34%   +2.35%     
+ Complexity     6227     4794    -1433     
============================================
  Files           833      592     -241     
  Lines         31694    23078    -8616     
  Branches       5265     3836    -1429     
============================================
- Hits          23452    17620    -5832     
+ Misses         5549     3407    -2142     
+ Partials       2693     2051     -642     
Impacted Files Coverage Δ Complexity Δ
...ozilla/components/feature/prompts/PromptFeature.kt 78.93% <0.00%> (-0.25%) 65.00 <0.00> (ø)
.../components/browser/engine/gecko/ext/CreditCard.kt 100.00% <100.00%> (ø) 0.00 <0.00> (?)
...browser/engine/gecko/prompt/GeckoPromptDelegate.kt 89.69% <100.00%> (+0.49%) 46.00 <1.00> (+1.00)
...lla/components/concept/engine/prompt/CreditCard.kt 100.00% <100.00%> (ø) 7.00 <7.00> (?)
.../components/concept/engine/prompt/PromptRequest.kt 79.09% <100.00%> (+0.99%) 0.00 <0.00> (ø)
...o/webnotifications/GeckoWebNotificationDelegate.kt 100.00% <0.00%> (ø) 5.00% <0.00%> (+2.00%)
...main/java/mozilla/components/service/fxa/Config.kt
...ce/digitalassetlinks/api/ListStatementsResponse.kt
...a/components/service/fxa/FxaDeviceConstellation.kt
...lla/components/feature/top/sites/TopSitesConfig.kt
... and 242 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 471da2b...bf725a2. Read the comment docs.

@mergify
Copy link
Contributor

mergify bot commented May 5, 2021

This pull request has conflicts when rebasing. Could you fix it @gabrielluong? 🙏

@gabrielluong gabrielluong force-pushed the 10205 branch 5 times, most recently from 95219e0 to 9316743 Compare May 9, 2021 16:46
*/
@Parcelize
data class CreditCard(
val guid: String?,
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems a bit strange to have an identifier that can be null? Would you mind helping me to clarify this part?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

That seems odd. Maybe there is a mistake in the API? The other fields are non-null, so a GUID should also be non-null.

Copy link
Contributor

@Amejia481 Amejia481 left a comment

Choose a reason for hiding this comment

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

Just a couple of nits and one question that doesn't block from landing.

- Implements an override function of `onCreditCardSelect` in `GeckoPromptDelegate`
- Adds a new `CreditCard` data class in `concept-engine`. This is a parallel of GV's
`Autocomplete.CreditCard`. We can't using the existing `CreditCard` from `concept-storage`
since that has encryption dependencies whereas the card number is already decrypted
when it reaches GV.
- Adds a new `SelectCreditCard` in `PromptRequest`
@gabrielluong gabrielluong added the 🛬 needs landing PRs that are ready to land label May 11, 2021
@mergify mergify bot merged commit b185faa into mozilla-mobile:master May 11, 2021
@gabrielluong gabrielluong deleted the 10205 branch May 11, 2021 03:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🛬 needs landing PRs that are ready to land 🕵️‍♀️ needs review PRs that need to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for select credit card prompt
3 participants