Import data from a google spreadsheet #903

devduarte · 2021-05-12T17:33:57Z

Add a new input field to enter a google spreadsheet link.
Add a new endpoint to retrieve the spreadsheet link
Create a csv file importing the spreadsheet data
Continue the csv import process as always using the created csv file in the previous step

…orm to paste the spreadsheet url

devduarte · 2021-05-12T17:47:43Z

app/views/import_wizards/index.haml

+        %form#upload_form{action: upload_csv_collection_import_wizard_path(collection), method: :post, enctype: 'multipart/form-data', style: 'display: inline-block'}
+          %input{type: :hidden, name: 'authenticity_token', value: form_authenticity_token}
+          %div
+            .icon.fexport.black Upload a CSV file to update multiple sites
+            %input#upload{type: :file, name: :file, style: 'visibility:hidden; width:1px; height: 1px'}
+        %form#upload_form_spreadsheet{action: import_csv_from_google_spreadsheet_collection_import_wizard_path(collection), method: :post, enctype: 'multipart/form-data', style: 'display: inline-block'}
+          %input{type: :hidden, name: 'authenticity_token', value: form_authenticity_token}
+          .icon.syes_no.black{style: 'cursor: pointer;'} or add a spreadsheet url
+          %input{type: 'text', id: :spreadSheetLink, name: 'spreadSheetLink', placeholder: 'Enter url', style: 'width: 300px;'}


I was not very sure where to put these css properties since there are not a lot stylesheets files. I know inline styles are not a good practice but since they were being used across several places in the application, I kind of followed the same rule.

It seems to be ResourceMap's way - it's OK 👍

matiasgarciaisaia

I'd rather add some documentation and error handling before merging. The feature looks nice, though :)

matiasgarciaisaia · 2021-05-12T20:08:44Z

app/controllers/import_wizards_controller.rb

+  end
+
+  def from_google_spreadsheet(spread_sheet_link)
+    filename = "#{current_user.id}_#{collection.id}.csv"


Does this work if the same user imports a spreadsheet into a collection after having imported a previous one? What potential conflicts could arise then?

The file's content gets overwritten with the spreadsheet content that the user inputs. Filename does not change, but content does. We could use the spreadsheet id as the filename, but then we will have stored a csv file for every single spreadsheet users want to import.
With the current code we will have one spreadsheet per user that used the feature as the worst case scenario.

matiasgarciaisaia · 2021-05-12T20:10:14Z

app/models/spreadsheet_service.rb

+class SpreadsheetService
+  def self.get_data(spreadsheet_id)
+    service = Google::Apis::SheetsV4::SheetsService.new
+    service.key = ENV['GOOGLE_SHEET_API_KEY']


Let's add some documentation regarding this API Key (how to get it, what configurations may be needed).

matiasgarciaisaia · 2021-05-12T20:13:27Z

app/models/spreadsheet_service.rb

+
+  def self.get_range(spreadsheet_id)
+    service = Google::Apis::SheetsV4::SheetsService.new
+    service.key = ENV['GOOGLE_SHEET_API_KEY']


Let's take a look at how other configurations are used on the app.

Rails has the Settings global object that allows users to configure the app via different mechanisms (the different environments' yml files, environment variables, etc). Instead of only supporting environment variables, let's use that. And we avoid having direct ENV access from the business code - which is harder to track.

matiasgarciaisaia · 2021-05-12T20:42:31Z

app/models/spreadsheet_service.rb

+    begin
+      response = service.get_spreadsheet_values(spreadsheet_id, range)
+    rescue Exception => e
+      raise ActionController::BadRequest.new(), e.message()


I guess we need to add some error handling regarding the Spreadsheet import - catch the exception, log it, show some error to the user asking to contact the site admin?

matiasgarciaisaia · 2021-05-12T20:43:55Z

app/views/import_wizards/index.haml

+    $(".fexport").on("click", function(){
+      document.getElementById('upload').click();
+    });
+    $(".syes_no").on("click", function(){
+      $('#upload_form_spreadsheet').submit();
+    });


Can we add IDs to these clickable elements? .fexport is referring to a CSS class, which may be applied to multiple elements - but we probably don't want other .fexports to trigger this very same action.

matiasgarciaisaia · 2021-05-12T20:44:39Z

app/views/import_wizards/index.haml

+        %form#upload_form{action: upload_csv_collection_import_wizard_path(collection), method: :post, enctype: 'multipart/form-data', style: 'display: inline-block'}
+          %input{type: :hidden, name: 'authenticity_token', value: form_authenticity_token}
+          %div
+            .icon.fexport.black Upload a CSV file to update multiple sites
+            %input#upload{type: :file, name: :file, style: 'visibility:hidden; width:1px; height: 1px'}
+        %form#upload_form_spreadsheet{action: import_csv_from_google_spreadsheet_collection_import_wizard_path(collection), method: :post, enctype: 'multipart/form-data', style: 'display: inline-block'}
+          %input{type: :hidden, name: 'authenticity_token', value: form_authenticity_token}
+          .icon.syes_no.black{style: 'cursor: pointer;'} or add a spreadsheet url
+          %input{type: 'text', id: :spreadSheetLink, name: 'spreadSheetLink', placeholder: 'Enter url', style: 'width: 300px;'}


It seems to be ResourceMap's way - it's OK 👍

…n error message to the user.

…n those ids and not on icon classes

The code had the wrong identation, so the upload controls were only shown for empty collections. This commit moves the code to where it belongs, so only the Download section changes depending on having sites or not.

So we can easily configure it on Rancher/Docker

matiasgarciaisaia

Great job!

matiasgarciaisaia · 2021-05-14T14:25:31Z

app/views/import_wizards/index.haml

+      %div
+        %form#upload_form{action: upload_csv_collection_import_wizard_path(collection), method: :post, enctype: 'multipart/form-data', style: 'display: inline-block'}
+          %input{type: :hidden, name: 'authenticity_token', value: form_authenticity_token}
+          %div
+            #upload_icon_csv.icon.fexport.black Upload a CSV file to update multiple sites
+            %input#upload{type: :file, name: :file, style: 'visibility:hidden; width:1px; height: 1px'}
+        %form#upload_form_spreadsheet{action: import_csv_from_google_spreadsheet_collection_import_wizard_path(collection), method: :post, enctype: 'multipart/form-data', style: 'display: inline-block'}
+          %input{type: :hidden, name: 'authenticity_token', value: form_authenticity_token}
+          #upload_icon_spreadsheet.icon.syes_no.black{style: 'cursor: pointer;'} or add a spreadsheet url
+          %input{type: 'text', id: :spreadSheetLink, name: 'spreadSheetLink', placeholder: 'Enter url', style: 'width: 300px;'}
+
+


MEGA fail 😂.

The %div (with the %forms and all) got moved inside the -else clause of the -if. So if you already have sites in your collection, you won't see the Upload section anymore.

I hadn't notice it in my previous review, either.

So we should basically unindent this whole block. I've just fixed that.

Ahaha, nice catch! Thanks for the fix! :D

See instedd/resourcemap#903

They are breaking, probably due to the changes in #903 being incompatible with old versions of Firefox - the engine is not able to find the upload <input> control.

Diego added 4 commits May 12, 2021 13:24

Add google api client gem and spreadsheet service model

41d18a6

Add csv import from a google spreadsheet endpoint and fields in the f…

363b4f1

…orm to paste the spreadsheet url

Delete open3 require from import_wizards_controller.rb

114303a

Add params validation before importing from a google spreadsheet

5262725

devduarte requested a review from matiasgarciaisaia May 12, 2021 17:33

Add placeholder to spreadsheet url input and make it wider

334fb2c

devduarte commented May 12, 2021

View reviewed changes

devduarte self-assigned this May 12, 2021

matiasgarciaisaia reviewed May 12, 2021

View reviewed changes

devduarte and others added 5 commits May 13, 2021 10:53

Add google sheets api key documentation in README.md

f48d28c

Catch exception when importing from google sheets fails and display a…

4be22e1

…n error message to the user.

Add ids to clickable icons and change onclick functions to be based o…

06af00a

…n those ids and not on icon classes

Fix Import Wizard forms

22c2b3e

The code had the wrong identation, so the upload controls were only shown for empty collections. This commit moves the code to where it belongs, so only the Download section changes depending on having sites or not.

Support setting Google Sheets API Key via environment

e57750b

So we can easily configure it on Rancher/Docker

matiasgarciaisaia approved these changes May 14, 2021

View reviewed changes

devduarte merged commit 82bc51b into master May 14, 2021

matiasgarciaisaia added a commit to instedd/rancher-catalog that referenced this pull request May 21, 2021

ResourceMap: Add GOOGLE_SHEET_API_KEY setting

f1450fb

See instedd/resourcemap#903

matiasgarciaisaia added a commit that referenced this pull request Nov 12, 2021

Skip import wizard integration tests

f2f4695

They are breaking, probably due to the changes in #903 being incompatible with old versions of Firefox - the engine is not able to find the upload <input> control.

matiasgarciaisaia added a commit that referenced this pull request Nov 15, 2021

Skip import wizard integration tests

93935b8

They are breaking, probably due to the changes in #903 being incompatible with old versions of Firefox - the engine is not able to find the upload <input> control.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Import data from a google spreadsheet #903

Import data from a google spreadsheet #903

devduarte commented May 12, 2021

devduarte May 12, 2021

matiasgarciaisaia May 12, 2021

matiasgarciaisaia left a comment

matiasgarciaisaia May 12, 2021

devduarte May 13, 2021

matiasgarciaisaia May 12, 2021

matiasgarciaisaia May 12, 2021

matiasgarciaisaia May 12, 2021

matiasgarciaisaia May 12, 2021

matiasgarciaisaia May 12, 2021

matiasgarciaisaia left a comment

matiasgarciaisaia May 14, 2021

devduarte May 14, 2021

Import data from a google spreadsheet #903

Import data from a google spreadsheet #903

Conversation

devduarte commented May 12, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matiasgarciaisaia 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

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

matiasgarciaisaia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment