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

Emailed CSV reports (generated on server/transact platform async) should be localized to merchant/store language - CSV language modal only works first time #9737

Open
csmcneill opened this issue Nov 15, 2024 · 11 comments
Assignees
Labels
category: projects For any issues which are part of any project, including bugs, enhancements, etc. focus: reporting priority: medium The issue/PR is medium priority—non-critical functionality loss, minimal effect on usability type: bug The issue is a confirmed bug.

Comments

@csmcneill
Copy link
Contributor

csmcneill commented Nov 15, 2024

Describe the bug

#7938 introduced localized CSV information. Currently, if a site's language is not set to English (United States):

  1. No localization prompt renders
  2. CSVs are delivered in English

I tried reverting to older versions of WooPayments and WooCommerce core without success. This leads me to believe that recent changes in WordPress 6.7 and how it handles localization might be the underlying cause.

Note

CSVs downloaded directly from wp-admin (e.g., fewer than 25 items) are localized as expected.

cc @mordeth

To Reproduce

Update: localisation seems to only work for the CSV generated immediately after modal flow - i.e. it only works once

How to reproduce
  • Set site language to non-English.
  • Go to transactions or other list view with > 25 items (bug only happens with server-rendered CSVs).
  • Click Download.
  • If you haven't already set CSV download preference, you should see this modal:

Image

  • Select the non-English option.
    • Remember the language settings. should be checked, leave it checked (assuming this matters – maybe not relevant).
  • Click Download.
  • Check your email, you should see
    • Non-English email
    • Non-English CSV column headers

Image
Image

  • Now reload the transactions view (or other list view), and click Download again
  • (No language prompt modal)
  • Check your email
    • The email and CSV are in English again
Original reproduce steps
  1. Access wp-admin on a site that has 25+ transactions.
  2. Navigate to Settings > General.
  3. Change the site language to Español.
  4. Navigate to Dashboard > Updates.
  5. Select the Update languages button (which may appear as Actualizar las traducciones).
  6. Navigate to Payments > Settings.
  7. Confirm that the Reporting section is visible.
  8. Navigate to Payments > Transactions.
  9. Confirm that there are 25 or more transactions.
  10. Select the Download option to trigger a CSV report sent via email.
  11. Note that no modal appears.
  12. Navigate to your email inbox.
  13. Open the Your WooPayments Export email.
  14. Download the CSV.
  15. Open the CSV.
  16. Note that the contents of the CSV are not translated.

Actual behavior

Emailed CSVs are not localized, and the process for generating localized CSV reports does not work as documented here.

Expected behavior

Downloading reports that are 25+ items prompt the user to select their preferred language, and CSV files are delivered via email

Additional context

No user reports AFAIK. Found this issue while performing other tests.

@csmcneill csmcneill added the type: bug The issue is a confirmed bug. label Nov 15, 2024
@haszari haszari changed the title Emailed CSV reports are no longer localized Emailed CSV reports (generated on server/transact platform async) should be localized to merchant/store language Dec 5, 2024
@haszari haszari added the priority: medium The issue/PR is medium priority—non-critical functionality loss, minimal effect on usability label Dec 5, 2024
@haszari
Copy link
Contributor

haszari commented Dec 5, 2024

CSVs downloaded directly from wp-admin (e.g., fewer than 25 items) are localized as expected.

To clarify, we have two different CSV rendering code paths depending on the number of rows – plugin and server/service.

I suspect what's happening here is …

  • The server-generated CSVs don't have access to the store language, or a bug has crept in there.
  • Plugin-generated CSVs have access to all store settings so they are localised consistently.

I think we should fix this (in server) since most merchants will have lots of rows to export.

Long term, I think we should focus on server-generated CSVs and remove the plugin implementation. That will be less effort to maintain and more consistent user experience.

@brucealdridge
Copy link
Contributor

No localization prompt renders

@csmcneill Can you explain what this is or provide a screenshot of what you expect to see here.

Are the server sent emails localized?

@haszari
Copy link
Contributor

haszari commented Dec 8, 2024

Can you explain what this is or provide a screenshot of what you expect to see here.

Are the server sent emails localized?

Based on #7938, looks like yes :)

FYI @brucealdridge there's good info and a screenshot of an csv language modal in the linked PR from @mordeth :

(TIL!)

@jessy-p jessy-p self-assigned this Dec 9, 2024
@haszari
Copy link
Contributor

haszari commented Dec 10, 2024

@csmcneill I've tried reproducing this bug without success! Looking at the overall flow for this, it is IMO a little confusing, so I wouldn't be surprised if you hit an edge case.

I'm keen to rework this code but before we do that we should confirm if there is an actual bug and ensure we can reproduce it. @jessy-p have you had any luck reproducing?

Here are some notes from my investigation.

  • The language prompt (modal) is only shown if:
    • The site language is not the "default" language (hard coded US english). Note this is the WordPress setting, not payments account - aka the UI language.
    • The modal has not previously been "dismissed".
    • IMO this is a little subtle and complex – we have a cool UI for selecting a language, but US-english testers or merchants never get to use it. It might be off our radar.

if ( 'endpoint' === downloadType ) {
if ( ! isDefaultSiteLanguage() && ! isExportModalDismissed() ) {
setCSVExportModalOpen( true );
} else {

export const isDefaultSiteLanguage = () => {
if ( typeof wcpaySettings === 'undefined' ) {
return true;
}
return wcpaySettings.locale?.code === 'en_US';
};

export const isExportModalDismissed = () => {
if ( typeof wcpaySettings === 'undefined' ) {
return true;
}
return wcpaySettings?.reporting?.exportModalDismissed ?? false;
};

  • I can't find anywhere that exportModalDismissed is set. This would mean the modal shows every time, i.e. opposite bug to what's reported.

    • Maybe this setting should be something like:
      • Language to use for CSVs: US | whatever the site language is set to | specify a language to always use | ask me every time I export. I think exportModalDismissed is a version of the "ask me every time", i.e. maybe the design is that you only see this modal once.
  • The modal says Export transactions report in: [language], but the setting applies to all CSV exports. A minor point but it could be confusing.

  • The code to wrangle the modal and server vs plugin CSV (etc) is replicated in all the different list view components. This is not necessarily a problem, but if there is a bug, it might affect one specific report only.

  • My technique for testing the modal + server CSV flow was to hack const downloadType = 'endpoint'; in a list view. When I do this, the flow works, but I am never emailed a CSV. So I haven't yet tested if the language settings were applied to the CSV. I suspect this is not a real bug, i.e. I need to kick my server background jobs or it returns early for small row counts.

  • The translations we use for the server-generated CSV need to come from somewhere. How does this work? Is it possible we don't have access to translation files, or have a bug with localising on server?

I think we're overcomplicating this!

Maybe we could simplify:

  • All CSVs are always exported in site language.
  • If merchant wants US English (previous default behaviour), they can temporarily reset site language (admin) as a workaround. I don't think it's worth building a complex UI to support this edge case.

We could potentially keep the site setting – i.e. a preference for language to export CSVs in – and a version of the modal – but I'm not sure it's worthwhile. I would expect the majority of merchants want CSVs in the same language as UI.

@csmcneill
Copy link
Contributor Author

@haszari Thank you for digging into this!

While I can't comment on how it should work, I can only confirm how it currently works on the Atomic sites I've tested this on vs. how it worked whenever the change was first introduced, especially since I was heavily involved in testing its implementation.

It is possible that a value was set for exportModalDismissed on the sites I used for testing, but I recollect that this modal always appeared (which matches your estimation) regardless of whether I selected Cancel or Download.

Perhaps @aheckler can test this as well?

@haszari
Copy link
Contributor

haszari commented Dec 11, 2024

I can only confirm how it currently works on the Atomic sites I've tested this on vs. how it worked whenever the change was first introduced, especially since I was heavily involved in testing its implementation.

Strange. I just had another crack at it.

  • Testing on my pressable test store. It is up to date:

    • WooCommerce Payments 8.6.0
    • WooCommerce 9.4.3
    • WordPress 6.7.1
  • When I set my site language to English (New Zealand), I don't get a language modal.

  • When I set my site language to Espanõl

    • I get a language modal
    • I choose Espanõl (interestingly, not the default)
    • I see toast notifications saying that
      • My csv will be emailed
      • My settings were saved
      • (the order of these seems reversed – should save settings, then email CSV?)
    • The email and CSV are in Spanish 🎉

So I'm still baffled!

Or not …

Note that I set (English NZ) and reset my site language here – at this point it is Espanõl.

  • When I reload the transactions list page
  • And do another export
  • The CSV and email are in English 🚬

So possibly the bug is…

  • The modal works the first time
  • And from then on you get no modal and an English/default CSV?

I'll add this to the description/title, assuming that's reproducible :)

@haszari haszari changed the title Emailed CSV reports (generated on server/transact platform async) should be localized to merchant/store language Emailed CSV reports (generated on server/transact platform async) should be localized to merchant/store language - CSV language modal only works first time Dec 11, 2024
@aheckler
Copy link
Member

I tried to repro using the steps in the OP but could not. The second time through I went to export the transactions, saw no modal, but the email and CSV were in German (the non-English language I'd chosen).

@haszari
Copy link
Contributor

haszari commented Dec 11, 2024

The mystery deepens!

@jessy-p
Copy link
Contributor

jessy-p commented Dec 11, 2024

The modal works the first time
And from then on you get no modal and an English/default CSV?

This seems to be reliable way to replicate the bug. Dismiss the modal, after that, changing the option from settings seems to have no effect. Whatever was set at the time the modal was dismissed is stuck. Seems to be an interaction between option wcpay_reporting_export_modal_dismissed and reporting_export_language or some cache.

I can't find anywhere that exportModalDismissed is set.

It is set in wcpay_reporting_export_modal_dismissed option.

@haszari
Copy link
Contributor

haszari commented Dec 11, 2024

@jessy-p perhaps we should explore radically simplifying this:

  • All CSVs export using site language setting.
  • Plugin sends current site language with CSV request.
  • No modal or settings.

By default we'd have to fix the bugs in the current design, but I think it's better to zoom out and keep it super basic (or as a fallback, adjust the existing design so it's simpler).

If you think that's worth pursuing, could you P2 a proposal? Also include:

  • Find out who we'd need to make the decision (e.g. folks involved in current implementation, product / engineering leadership etc).
  • What data we might need to make a decision.

@jessy-p
Copy link
Contributor

jessy-p commented Dec 12, 2024

Published here: pdjTHR-4sV-p2

@haszari haszari added the category: projects For any issues which are part of any project, including bugs, enhancements, etc. label Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: projects For any issues which are part of any project, including bugs, enhancements, etc. focus: reporting priority: medium The issue/PR is medium priority—non-critical functionality loss, minimal effect on usability type: bug The issue is a confirmed bug.
Projects
None yet
Development

No branches or pull requests

6 participants