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 bug with checking user preferences for report column visibility. #2806

Merged
merged 1 commit into from Aug 19, 2019

Conversation

@jeffstieler
Copy link
Contributor

commented Aug 16, 2019

(Found while reproducing #2719)

Fixes a bug introduced in 7ecfb1d#diff-cbe713361af618d887a218e657fcc463R53.

When there are no user preferences for column visibility on a report, the value is an empty string. Since we're storing the hidden columns, if a user chooses to show all columns, we store an empty array.

The above linked commit introduced a .length check which caused "don't hide anything" preferences to revert to the column default visibility.

The easiest way to see this in the Products report - SKU column.

This PR fixes the issue by removing the .length check and introducing a new visible object property rather than overloading the hiddenByDefault property.

Detailed test instructions:

  • On master
  • Go to Analytics > Products
  • Toggle all columns to be visible
  • Refresh page
  • Verify the SKU column is hidden upon refresh
  • Check out this branch
  • Refresh
  • Verify SKU column is visible

Changelog Note:

Fix: report column visibility preference bug.

@jeffstieler jeffstieler requested a review from woocommerce/wc-admin Aug 16, 2019

@jeffstieler jeffstieler added this to In Progress PRs (for automation purposes) in wc-admin via automation Aug 16, 2019

@jeffstieler jeffstieler force-pushed the fix/report-column-visibility branch from 0585872 to fec4938 Aug 16, 2019

@rrennick
Copy link
Collaborator

left a comment

@jeffstieler Nice catch :shipit:

@jeffstieler jeffstieler merged commit 57c852e into master Aug 19, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

wc-admin automation moved this from In Progress PRs (for automation purposes) to Done Sprint 23 (August 13 - August 26) Aug 19, 2019

@jeffstieler jeffstieler deleted the fix/report-column-visibility branch Aug 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.