[#9] [Backend] As a user, I can upload a CSV file, [#12] [UI] As a user, I can upload a CSV file · Pull Request #39 · nimblehq/google-scrapper-ruby

ghost · 2023-05-30T10:07:29Z

Closes

What happened 👀

User can visit /search_stats/new to upload a CSV file.

Insight 📝

Maximum 1,000 keywords per file.

Proof Of Work 📹

Release - 0.1.0

Co-authored-by: Long Nguyen <nguyenduclong@msn.com>

Co-authored-by: Sang Huynh Thanh <63148598+sanG-github@users.noreply.github.com>

Release/0.2.0

…ure/upload-file

github-actions · 2023-06-01T11:44:56Z

app/controllers/search_stats_controller.rb

+    render :new, locals: { search_stat: search_stat }
+  end
+
+  def create


⚠️ Assignment Branch Condition size for create is too high. [<5, 21, 6> 22.41/17]

github-actions · 2023-06-01T11:44:57Z

app/controllers/search_stats_controller.rb

+    render :new, locals: { search_stat: search_stat }
+  end
+
+  def create


⚠️ Method has too many lines. [11/10]

@topnimble don't just skip the warnings from the linter, the checks are imposed for a reason

github-actions · 2023-06-01T11:44:58Z

app/controllers/search_stats_controller.rb

+    render :new, locals: { search_stat: search_stat }
+  end
+
+  def create


⚠️ Has approx 10 statements

github-actions · 2023-06-01T11:44:59Z

app/controllers/search_stats_controller.rb

+  end
+
+  def create
+    csv_file = params[:csv_file]


⚠️ Calls 'params[:csv_file]' 2 times

github-actions · 2023-06-01T11:45:01Z

app/controllers/search_stats_controller.rb

+
+    raise 'Invalid file type' unless csv_file.content_type == ALLOWED_MIME_TYPE
+
+    csv_file_content = params[:csv_file].read


⚠️ Calls 'params[:csv_file]' 2 times

github-actions · 2023-06-01T11:45:02Z

spec/requests/search_stats_spec.rb

+RSpec.describe 'Search Stats', type: :request do
+  describe 'POST #create' do
+    context 'given a valid file' do
+      it 'inserts keywords to the database and redirects to search stat index' do


⚠️ Example has too many lines. [6/5]

Please change the rule of this one, many tests will take more than 5 lines. 💭

longnd · 2023-06-06T11:17:33Z

app/controllers/search_stats_controller.rb

+    csv_file = params[:csv_file]
+
+    raise 'Invalid file type' unless csv_file.content_type == ALLOWED_MIME_TYPE
+
+    csv_file_content = params[:csv_file].read
+    keywords = CSV.parse(csv_file_content).flatten.compact_blank
+
+    raise 'Invalid file data' unless keywords.any?
+    raise 'Too many keywords' unless keywords.count <= MAXIMUM_KEYWORDS


The file validation is not the reponsibility of the controller, it usually handled in the model.
In this case, we can use create a form to handle both validation and parsing the file.
https://nimblehq.co/compass/development/code-conventions/ruby/ruby-on-rails/#forms

also, the validations here can be extracted to a separated validator to used in the form
https://nimblehq.co/compass/development/code-conventions/ruby/ruby-on-rails/#validators

longnd · 2023-06-06T11:23:46Z

app/controllers/search_stats_controller.rb

+    render :new, locals: { search_stat: search_stat }
+  end
+
+  def create


@topnimble don't just skip the warnings from the linter, the checks are imposed for a reason

longnd · 2023-06-06T11:26:32Z

spec/requests/search_stats_spec.rb

+      end
+    end
+
+    context 'given too many keywords' do


how many is too many? Let's be specific

Suggested change

context 'given too many keywords' do

context 'Given that the file contains more keywords than allowed' do

sanG-github · 2023-06-07T09:37:16Z

app/controllers/search_stats_controller.rb

+  end
+
+  def create
+    csv_file = params[:csv_file]


As Long suggested, we should have a dedicated from/model/service to handle the uploaded file.

sanG-github · 2023-06-07T09:38:08Z

app/models/search_stat.rb


 class SearchStat < ApplicationRecord
  validates :keyword, presence: true
-  validates :raw_response, presence: true


Could you please point me out why we remove this validation? 🤔

sanG-github · 2023-06-07T09:40:09Z

spec/fixtures/files/invalid_type.txt

@@ -0,0 +1,3 @@
+1st keyword


About the invalid type, we can specify it when attaching the file to the Fabricator instead of using a dedicated file. 😉
But it's an additional option, you can use which way is more convenient.

Rack::Test::UploadedFile.new(Rails.root.join('spec', 'fixtures', 'files', 'keywords.csv'), 'plain/text')

sanG-github · 2023-06-07T09:41:59Z

spec/requests/search_stats_spec.rb

+RSpec.describe 'Search Stats', type: :request do
+  describe 'POST #create' do
+    context 'given a valid file' do
+      it 'inserts keywords to the database and redirects to search stat index' do


Please change the rule of this one, many tests will take more than 5 lines. 💭

sanG-github · 2023-06-07T09:45:34Z

spec/requests/search_stats_spec.rb

+
+        expect { post search_stats_path, params: params }.to raise_error(RuntimeError, 'Invalid file data')
+
+        expect(SearchStat.all.count).to eq(0)


With count, no need for .all anymore. (ref)

mosharaf13 and others added 29 commits May 22, 2023 17:15

Merge pull request #29 from mosharaf13/release/0.1.0

457bdbc

Release - 0.1.0

Add home page

33296d1

Add devise gem

7ad64d2

Add config files for devis gem

63209d9

Add alert in layout application

a941293

Add basic user registration flow using devise gem

f08d8da

Show welcome message to signed in users

dc0a701

Add password complexity validation for user signup

f767e8f

Change minimum password requirement from 6 to 8 in devise config

183d249

Remove redundant password length error

7539e8b

Fix lint

a72413f

Change branch name for fly deployment

53cc4ff

Remove nil check from password confirmation validation

3acfcb6

Add github co owners file

5ca8dce

Update Gemfile

60021d9

Co-authored-by: Long Nguyen <nguyenduclong@msn.com>

Change user email type to citext

ab783db

Extract devise configs as a authenticable module for user model

a22fac3

Extract authenticable module from user model

c41b8f6

Run password complexity validation on user create hook

0b86c77

Fix indent

5e75a50

Enable and disable "citext" extension on migration

db8150d

Remove render from home controller index

6158bf1

Co-authored-by: Sang Huynh Thanh <63148598+sanG-github@users.noreply.github.com>

Make password complexity checking private

5ea7ec4

Co-authored-by: Sang Huynh Thanh <63148598+sanG-github@users.noreply.github.com>

Update app/views/home/index.html.erb

fe8c199

Co-authored-by: Sang Huynh Thanh <63148598+sanG-github@users.noreply.github.com>

Fix lint

1213306

Update config/routes.rb

27e7f72

Co-authored-by: Sang Huynh Thanh <63148598+sanG-github@users.noreply.github.com>

Merge pull request #37 from mosharaf13/release/0.2.0

380c5b0

Release/0.2.0

Upload file

f095c0a

Insert to database

1065845

ghost self-assigned this May 30, 2023

Merge remote-tracking branch 'origin/backend/list-keywords' into feat…

e8851a2

…ure/upload-file

ghost changed the base branch from main to backend/list-keywords June 1, 2023 09:50

topnimble added 7 commits June 1, 2023 16:56

Update tests

565303c

Update code

2ce0def

Update tests

4925fd2

Update code

93d1548

Update seeds

9c29c04

Insert current user ID

4895f2e

Remove validation

215cc21

ghost marked this pull request as ready for review June 1, 2023 11:41

ghost requested review from longnd, mosharaf13 and sanG-github as code owners June 1, 2023 11:41

ghost added the feature label Jun 1, 2023

github-actions bot reviewed Jun 1, 2023

View reviewed changes

ghost added the @0.3.0 label Jun 1, 2023

ghost added this to the 0.3.0 milestone Jun 1, 2023

Update style

91e5b47

Base automatically changed from backend/list-keywords to develop June 2, 2023 10:05

mosharaf13 added the @0.4.0 label Jun 6, 2023

longnd reviewed Jun 6, 2023

View reviewed changes

sanG-github reviewed Jun 7, 2023

View reviewed changes

mosharaf13 approved these changes Jun 12, 2023

View reviewed changes


		raise 'Invalid file type' unless csv_file.content_type == ALLOWED_MIME_TYPE

		csv_file_content = params[:csv_file].read

	context 'given too many keywords' do
	context 'Given that the file contains more keywords than allowed' do


		expect { post search_stats_path, params: params }.to raise_error(RuntimeError, 'Invalid file data')

		expect(SearchStat.all.count).to eq(0)

Conversation

ghost commented May 30, 2023 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What happened 👀

Insight 📝

Proof Of Work 📹

Uh oh!

github-actions bot Jun 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jun 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

longnd Jun 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jun 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jun 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jun 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jun 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

longnd Jun 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ghost commented May 30, 2023 •

edited by ghost

Loading

github-actions bot Jun 1, 2023 •

edited

Loading

github-actions bot Jun 1, 2023 •

edited

Loading

longnd Jun 6, 2023 •

edited

Loading

github-actions bot Jun 1, 2023 •

edited

Loading

github-actions bot Jun 1, 2023 •

edited

Loading

github-actions bot Jun 1, 2023 •

edited

Loading

github-actions bot Jun 1, 2023 •

edited

Loading

longnd Jun 6, 2023 •

edited

Loading