Automattic / woocommerce-payments

Accept payments via credit card. Manage transactions within WordPress.
https://wordpress.org/plugins/woocommerce-payments/
Other
173 stars 69 forks source link

Fraud protection setting reverts back to Basic despite saving as "Advanced" #6497

Closed kaushikasomaiya closed 5 months ago

kaushikasomaiya commented 1 year ago

Original description

6363653-zen

It seems like when we switch to Advanced settings, the option in DB gets updated:

![E9Dky7.png](https://github.com/Automattic/woocommerce-payments/assets/60988591/eba9b6df-23aa-4bac-83d9-95e3ee7e401b) Full Size: E9Dky7.png

However, on re-loading the WCPay settings page again, the radio defaults to Basic and in a couple of seconds after the page load - the option reverts back to Basic

![ZygsMG.png](https://github.com/Automattic/woocommerce-payments/assets/60988591/58600f5f-2fe1-49c4-b0d9-2d2dec78d1eb) Full Size: ZygsMG.png

Same behavior is seen when "Configure" button is pressed and no settings are toggled on.

The advanced option only stays saved successfully if at least one filter is enabled, example: AVS.


Updated issue description

The original description of the issue is the expected behavior. If no advanced rules are toggled on when the 'Save changes' button is clicked, then no changes have been saved as there are no changes. Upon the settings page being reloaded the fraud protection setting will correctly show that it is still set to Basic settings.

The issue we need to fix is that we provide incorrect notifications to the merchant when they save settings on the Advanced Settings page when no advanced settings have been toggled on. When that happens we provide the following two incorrect notifications:

Those should only display when the merchant toggled one or more advanced rules on and then clicks save settings.

To fix this issue, we will make the following changes. First, we will update the Advanced Settings page to have the following text displayed below the breadcrumb/title:

At least one risk filter needs to be enabled for advanced protection.

Next, we'll set up a conditional check that runs when 'Save changes' button is clicked. That conditional will check to see if all the advanced rule settings are toggled off. If that check returns true, then we won't show the two success related notifications mentioned above, but instead will show the following notification:

At least one risk filter needs to be enabled for advanced protection.

Acceptance criteria

Screenshots

323347821-b4ffcd98-130d-4e98-8161-bb124489bf77

Screenshot showing the new line of text below the breadcrumb/title line.

324593354-9f9a0237-5473-47bc-90bf-1434cb52c33a

Screenshot showing the two success notifications that should be replaced with a single notification that displays the following text: At least one risk filter needs to be enabled for advanced protection.

zmaglica commented 1 year ago

This issue impacts Fraud detection, so assigning to team Axiom Sigma (based on team responsibilities) @LCmry @dwainm . Assigning as part of Gamma Triage process PcreKM-yM-p2

sverleis commented 7 months ago

Customer experiencing this issue: 7783354-zen

eduardoumpierre commented 7 months ago

If the merchant does not change any rules and saves the changes on the advanced settings page, the setting will be the same as the basic configuration, and that's the logic that is currently in place. We need to find a way to distinguish between FRT configurations that should be basic and those that should be advanced, even if no changes have been made.

eduardoumpierre commented 7 months ago

I have moved this ticket to "needs refinement" for the team to discuss a possible solution.

eduardoumpierre commented 7 months ago

Hey @elizaan36, could you please review this flow? Currently, if the merchant does not make any changes on the advanced settings page and save the changes, it will be displayed as "Basic" protection level on the WooPayments settings page. This is expected as the Basic protection level does not contain any additional rules.

elizaan36 commented 7 months ago

The UX for this needs a much closer look but we could solve this in the interim with an inline notice in the section after saving. "At least one filter needs to be enabled for Advanced protection".

I can share a mockup next week but this is on the design teams radar to take a closer look at the flow overall.

elizaan36 commented 6 months ago

Could a dismissible inline notice like this work? We're using this component elsewhere in WooPayments.

image

It should dismiss automatically when refreshing the page.

eduardoumpierre commented 6 months ago

@elizaan36 Thanks for the mockup and the solution idea!

I think this issue has a great impact on the "Advanced Fraud Settings" page since the changes done there can make the Risk Level shown as "Basic" on the regular settings page.

If the merchant undos the changes or doesn't make changes at all, the result will be that it will be marked as "Basic" since the ruleset configuration is the same as the "Basic" protection level.

Do you think this is something that we could communicate on the Advanced Fraud Settings page?

elizaan36 commented 5 months ago

Yes definitely! That may be better actually. Rather than the notice after the fact, that line of text can be added on the Advanced Fraud Settings page, below the breadcrumb/title. How does that sound?

image
allie500 commented 5 months ago

Yes definitely! That may be better actually. Rather than the notice after the fact, that line of text can be added on the Advanced Fraud Settings page, below the breadcrumb/title. How does that sound?

I like that idea. The other thing I would point out is that if you don't make any changes on the Advanced Settings page but click the save changes button you get two notifications displayed in the lower left corner (see screenshot below):

@elizaan36 what do you think if we put in a check when the save changes button is clicked on the Advanced Settings page? If no advanced risk filters are enabled then the above two notices are not displayed and instead a different notice is displayed with the text you suggest adding under the breadcrumb/title:

Screenshot 2024-04-22 at 3 46 14 PM (2)

elizaan36 commented 5 months ago

hey @allie500 Sorry this took me a bit to get back to you. I think that's a really great solution.

allie500 commented 5 months ago

hey @allie500 Sorry this took me a bit to get back to you. I think that's a really great solution.

No worries on the delay. I think we've got all the info we need now on how to address this issue and get it into our team's refined column. Thanks for your input and feedback.