Issue 3028 convert csv to excel #5209

xeniabarreto · 2023-09-15T02:17:00Z

What github issue is this PR for, if any?

Resolves #3028

What changed, and why?

I implemented data extraction via Excel format with automatic line break correction in the "Case Contact Notes" column.

As instructed by Shen Yang and Linda, I have also added a modal that allows users to choose the desired file format, either CSV or Excel.

How will this affect user permissions?

Supervisor permissions:
Admin permissions:

How is this tested? (please write tests!) 💖💪

I provide below a video demonstration of the changes.

Screenshots please :)

Gravacao.de.tela.de.14-09-2023.23.01.41.webm

Feelings gif (optional)

What gif best describes your feeling working on this issue? https://giphy.com/
How to embed:
![alt text](https://media.giphy.com/media/1nP7ThJFes5pgXKUNf/giphy.gif)

Feedback please? (optional)

We are very interested in your feedback! Please give us some :) https://forms.gle/1D5ACNgTs2u9gSdh9

…eneration.

chahmedejaz

Good work overall 👍
Edit: There are some failing specs. Please review if they are related to your changes. Thanks.

chahmedejaz · 2023-10-11T07:42:26Z

app/services/case_contacts_export_excel_service.rb

+    # NOTE: these header labels are for stakeholders and do not match the
+    # Rails DB names in all cases, e.g. added_to_system_at header is case_contact.created_at
+    {
+      internal_contact_number: case_contact&.id,


It's better to return an empty hash unless casa_contact present rather than using the safe navigation each time accessing this. i.e.

def self.DATA_COLUMNS(case_contact = nil) return {} unless casa_contact { internal_contact_number: case_contact.id, . . case_contact_notes: case_contact.notes } end

I'm in the process of learning, I'm very grateful for your review and suggestion.

@chahmedejaz I'd like to mention that I couldn't use your suggestion to return an empty hash if 'casa_contact' isn't present. The reason is that we're already using the same function with its keys for creating a modal where users select columns to filter before exporting. Thanks for your understanding :)

chahmedejaz · 2023-10-11T07:57:09Z

app/services/case_contacts_export_excel_service.rb

+    (string.to_s.size + 3) * font_scale
+  end
+
+  def self.DATA_COLUMNS(case_contact = nil)


I can see that this function is almost identical to the one we are using in case_contacts_export_csv_service.rb with format changes to added_to_system_at date:

casa/app/services/case_contacts_export_csv_service.rb

Lines 23 to 42 in 8f08a6a

def self.DATA_COLUMNS(case_contact = nil)

# Note: these header labels are for stakeholders and do not match the

# Rails DB names in all cases, e.g. added_to_system_at header is case_contact.created_at

{

internal_contact_number: case_contact&.id,

duration_minutes: case_contact&.report_duration_minutes,

contact_types: case_contact&.report_contact_types,

contact_made: case_contact&.report_contact_made,

contact_medium: case_contact&.medium_type,

occurred_at: I18n.l(case_contact&.occurred_at, format: :full, default: nil),

added_to_system_at: case_contact&.created_at,

miles_driven: case_contact&.miles_driven,

wants_driving_reimbursement: case_contact&.want_driving_reimbursement,

casa_case_number: case_contact&.casa_case&.case_number,

creator_email: case_contact&.creator&.email,

creator_name: case_contact&.creator&.display_name,

supervisor_name: case_contact&.creator&.supervisor&.display_name,

case_contact_notes: case_contact&.notes

}

end

It's better to have consistency (excel version of the hash looks better) across both CSV and Excel reports and extract it out in a common helper module to be included in both services.

@chahmedejaz Thank you for your suggestion!
I've implemented the changes as you recommended and pushed the modifications to the code.

chahmedejaz · 2023-10-27T05:01:59Z

Thank you so much for working on it, @xeniabarreto. Looks like we are almost there. Great work 👍

P.S. There are some failing checks. Please review if they are related to your changes. Thanks.

compwron · 2023-10-27T15:07:15Z

.allow_skipping_tests

@@ -39,6 +39,7 @@ policies/note_policy.rb
 presenters/base_presenter.rb
 presenters/case_contact_presenter.rb
 services/case_contacts_export_csv_service.rb
+services/case_contacts_export_excel_service.rb


Hmm I am not a fan of skipping tests...

I agree, but the methods are hard to test in isolation, and you can test them in the system spec.

compwron · 2023-10-27T15:07:38Z

app/models/case_contact_report.rb

@@ -11,6 +11,11 @@ def to_csv
    CaseContactsExportCsvService.new(case_contacts_for_csv, columns).perform
  end

+  def to_excel
+    case_contacts_for_excel = @case_contacts


do we need this extra variable?

compwron

It looks good! Let's get the CI build working

schoork · 2023-10-29T22:27:47Z

app/services/case_contacts_export_data_columns.rb

I liked how you extracted the same code here so both CSV and Excel services could use it.

But what I don't like (and predates your code) is how this is used for two totally different things:

To get the list of columns for the view

To get a hash for each row of the CSV/Excel file

This is why all the safe operators (&) because in the view it's sending in nil for the case contact. I think a better pattern is to have a list of the columns and use them in the decorator.

schoork · 2023-10-29T22:30:44Z

app/views/reports/index.html.erb

+  </div>
+</div>
+
+<script>


Not a fan of putting JS in a view. This should normally be separated into a JS file, but you can achieve the same result by moving the modal inside the form and letting the CSV and Excel buttons be your submit buttons. You just need to put name and value tags on the buttons.

schoork · 2023-10-29T22:33:18Z

app/services/case_contacts_export_excel_service.rb

+  def configure_case_contact_notes_width(sheet)
+    case_contact_notes_index = filtered_columns.index(:case_contact_notes)
+
+    if case_contact_notes_index


This was a good catch from the previous code. It was throwing a nil --> Integer error when the notes weren't a selected column.

some changes

skip service tests

github-actions · 2023-12-29T01:44:01Z

This PR has been open for a long time without any pushes or comments! What's up?

xeniabarreto added 6 commits September 5, 2023 09:38

feat add Excel data to existing CSV files.

ec3e610

Merge branch 'main' into issue-3028-convert-csv-to-excel

c5a318c

fix sheet.add_row

367637e

add modal to export csv/excel

fcd9eaf

feat formatted automatic line break column in "Case Contact Notes"

6c28b5d

chore: rubocop

7334fd4

xeniabarreto requested review from compwron, FireLemons and littleforest as code owners September 15, 2023 02:17

github-actions bot added ruby Pull requests that update Ruby code erb labels Sep 15, 2023

xeniabarreto added 2 commits September 15, 2023 11:31

chore: lint

f37d9a1

fix: update test message to match the latest changes for Excel file g…

4f58744

…eneration.

github-actions bot added the Tests! 🎉💖👏 label Sep 15, 2023

xeniabarreto added 3 commits September 15, 2023 11:45

chore: remove blank spaces

9110d41

chore: added file name for skipping test

9cf1ab4

chore: Fixed formatting issues in the case contact reports controller

819b320

chahmedejaz reviewed Oct 11, 2023

View reviewed changes

xeniabarreto added 2 commits October 26, 2023 09:51

fix close button Filter Columns modal

e7a1f0c

fix: refactor data columns

8c0207a

compwron reviewed Oct 27, 2023

View reviewed changes

compwron approved these changes Oct 27, 2023

View reviewed changes

schoork reviewed Oct 29, 2023

View reviewed changes

3028 Convert CSV to Excel //

510c33d

some changes

github-actions bot added the dependencies Pull requests that update a dependency file label Oct 29, 2023

3028 Convert CSV to Excel //

ff0b6c3

skip service tests

github-actions bot added the no-pr-activity label Dec 29, 2023

github-actions bot closed this Jan 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 3028 convert csv to excel #5209

Issue 3028 convert csv to excel #5209

xeniabarreto commented Sep 15, 2023

chahmedejaz left a comment •

edited

Loading

chahmedejaz Oct 11, 2023

xeniabarreto Oct 23, 2023

xeniabarreto Oct 26, 2023

chahmedejaz Oct 11, 2023

xeniabarreto Oct 26, 2023

chahmedejaz commented Oct 27, 2023 •

edited

Loading

compwron Oct 27, 2023

schoork Oct 29, 2023

compwron Oct 27, 2023

compwron left a comment

schoork Oct 29, 2023

schoork Oct 29, 2023

schoork Oct 29, 2023

github-actions bot commented Dec 29, 2023

	def self.DATA_COLUMNS(case_contact = nil)
	# Note: these header labels are for stakeholders and do not match the
	# Rails DB names in all cases, e.g. added_to_system_at header is case_contact.created_at
	{
	internal_contact_number: case_contact&.id,
	duration_minutes: case_contact&.report_duration_minutes,
	contact_types: case_contact&.report_contact_types,
	contact_made: case_contact&.report_contact_made,
	contact_medium: case_contact&.medium_type,
	occurred_at: I18n.l(case_contact&.occurred_at, format: :full, default: nil),
	added_to_system_at: case_contact&.created_at,
	miles_driven: case_contact&.miles_driven,
	wants_driving_reimbursement: case_contact&.want_driving_reimbursement,
	casa_case_number: case_contact&.casa_case&.case_number,
	creator_email: case_contact&.creator&.email,
	creator_name: case_contact&.creator&.display_name,
	supervisor_name: case_contact&.creator&.supervisor&.display_name,
	case_contact_notes: case_contact&.notes
	}
	end

Issue 3028 convert csv to excel #5209

Issue 3028 convert csv to excel #5209

Conversation

xeniabarreto commented Sep 15, 2023

What github issue is this PR for, if any?

What changed, and why?

How will this affect user permissions?

How is this tested? (please write tests!) 💖💪

Screenshots please :)

Feelings gif (optional)

Feedback please? (optional)

chahmedejaz left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chahmedejaz commented Oct 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

compwron left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 29, 2023

chahmedejaz left a comment •

edited

Loading

chahmedejaz commented Oct 27, 2023 •

edited

Loading