CSV reports: Grey out the option to add summary lines when csv is selected

[drummer83]

Description

No matter if you choose the add summary lines in CSV reports or not - the summary line will not be in the file.

Expected Behavior

• Add an explanation text that csv files will never have summary rows
• Grey out the option to add summary lines when csv is selected

Actual Behaviour

The option is there, but the summary lines are never.

Steps to Reproduce

1. Go to a report which has the option to add a summary line (e.g. Bulk Coop Reports)
2. Activate the checkbox for summary row
3. Create the report in CSV format
4. Look at the created file

Animated Gif/Screenshot

Workaround

Severity

bug-s3: a feature is broken but there is a workaround

Your Environment

• Version used:
• Browser name and version:
• Operating System and version (desktop or mobile):

[jibees]

It's intentional: rendering a summary (which has a particular formatting) seems to be impossible in json nor in csv (CSV file requires an equal number of columns on each lines).

I would vote to add an explanation about why it doesn't work, but it's intending.

wdyt @openfoodfoundation/train-drivers-product-owners ?

[drummer83]

I'm not part of Product, but my opinion: Yes, adding the explanation would be great. 👍 And from a usability point of view I would recommend to disable (grey out) the summary checkbox as soon as csv has been selected. We should change the order of appearance - I hope it's easy to do:

1. Report type
2. Generate reprt (on screen, csv, xlsx, ...)
3. Display (header/summary row)
4. Columns to hide

Mockup:

Makes sense?

EDIT: Same is true for header row, isn't it? If headers can't be included in the output we should deactivate that option.

[jibees]

yeap, sorry, same for header.

I really agree to your mockup, and I'll try to implement this, since it's making totally sense.

[RachL]

@jibees just FYI we know it's intended, it shouldn't have been opened as a bug indeed. But the option 1 was discussed on slack and it's how we went for disabling the checkboxes.

[jibees]

I'll unassign myself from this one at we've more urgent to do

[aintluks]

Hey guys, I've been trying this one. But there's something here that doesn't make sense to me because it's not working

On file app/views/admin/reports/_rendering_options.html.haml I've added the {'ng-model' => 'report_format'} here:

= select_tag :report_format, grouped_options_for_select({ | t('.formatted_data') => { t('.on_screen') => '', "PDF" => 'pdf', t('.spreadsheet') => 'xlsx' }, | t('.raw_data') => { "CSV" => 'csv' }, | }), {'ng-model' => 'report_format'}

and then here on the checkbox I added the {'ng-disabled' => "report_format == 'csv'"}

= check_box_tag :display_summary_row, true, params[:display_summary_row], {'ng-disabled' => "report_format == 'csv'"}

This to me makes sense but this won't work :/

I'm glad if you can help me with this @jibees

[jibees]

I don't think it's a good idea to add some angularjs here, as our global goal is to remove it, and remplace with StimulusJS (https://stimulus.hotwired.dev/). Would you mind please use StimulusJS for this one?