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

Remove MaskDataService to use hiding on rendering instead #12608

Open
rioug opened this issue Jun 25, 2024 · 0 comments
Open

Remove MaskDataService to use hiding on rendering instead #12608

rioug opened this issue Jun 25, 2024 · 0 comments

Comments

@rioug
Copy link
Collaborator

rioug commented Jun 25, 2024

What we should change and why (this is tech debt)

There is an enterprise setting that allows hiding Customer names in report see: Enterprises > Edit > Shop Preferences > Customer Names in Reports.
Currently this done via MaskDataService. It does so by updating the record in memory before rendering the report, this is potentially dangerous, what if the record get accidentally saved ? It is also a bit brittle, depending on how the data is ultimately loaded to render the report, it might just reload the "updated" record, thus not hiding anything.

A better solution is to hide the data at render time.We could use the existing functionality to achieve that:

ReportTemplate has a columns_format method :

It's used here in the ReportRowBuilder

def format_cell(value, column = nil)
return "none" if value.nil?
# Currency
if report.columns_format[column] == :currency
format_currency(value)
# Quantity
elsif report.columns_format[column] == :quantity && report.html_render?
format_quantity(value)
# Numeric
elsif report.columns_format[column] == :numeric
format_numeric(value)
# Percentage, a number between 0 and 1
elsif report.columns_format[column] == :percentage
format_percentage(value)
# Boolean
elsif value.in? [true, false]
format_boolean(value)
# Time
elsif value.is_a?(Time)
format_time(value)
# Date
elsif value.is_a?(Date)
format_date(value)
# Default
else
value
end
end

We could implement a "masked" format, which would return "hidden" based on "Customer Names in Reports" setting

The we can just add collumns_format in https://github.com/openfoodfoundation/openfoodnetwork/blob/master/lib/reporting/reports/orders_and_distributors/base.rb
ie:

def collumns_format
  {customer_name: :masked }
end

Context

This came up while fixing this a bug : #12594

Impact and timeline

@sigmundpetersen sigmundpetersen changed the title Remove MaskDataService to use hidding on rendering instead. Remove MaskDataService to use hiding on rendering instead Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: All the things 💤
Development

No branches or pull requests

1 participant