Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
457bdbc
Merge pull request #29 from mosharaf13/release/0.1.0
mosharaf13 May 22, 2023
33296d1
Add home page
mosharaf13 May 19, 2023
7ad64d2
Add devise gem
mosharaf13 May 19, 2023
63209d9
Add config files for devis gem
mosharaf13 May 19, 2023
a941293
Add alert in layout application
mosharaf13 May 19, 2023
f08d8da
Add basic user registration flow using devise gem
mosharaf13 May 19, 2023
dc0a701
Show welcome message to signed in users
mosharaf13 May 19, 2023
f767e8f
Add password complexity validation for user signup
mosharaf13 May 19, 2023
183d249
Change minimum password requirement from 6 to 8 in devise config
mosharaf13 May 19, 2023
7539e8b
Remove redundant password length error
mosharaf13 May 19, 2023
a72413f
Fix lint
mosharaf13 May 19, 2023
53cc4ff
Change branch name for fly deployment
mosharaf13 May 19, 2023
3acfcb6
Remove nil check from password confirmation validation
mosharaf13 May 19, 2023
5ca8dce
Add github co owners file
mosharaf13 May 22, 2023
60021d9
Update Gemfile
mosharaf13 May 22, 2023
ab783db
Change user email type to citext
mosharaf13 May 22, 2023
a22fac3
Extract devise configs as a authenticable module for user model
mosharaf13 May 22, 2023
c41b8f6
Extract authenticable module from user model
mosharaf13 May 22, 2023
0b86c77
Run password complexity validation on user create hook
mosharaf13 May 22, 2023
5e75a50
Fix indent
mosharaf13 May 22, 2023
db8150d
Enable and disable "citext" extension on migration
May 23, 2023
6158bf1
Remove render from home controller index
mosharaf13 May 25, 2023
5ea7ec4
Make password complexity checking private
mosharaf13 May 26, 2023
fe8c199
Update app/views/home/index.html.erb
mosharaf13 May 26, 2023
1213306
Fix lint
mosharaf13 May 26, 2023
27e7f72
Update config/routes.rb
mosharaf13 May 26, 2023
380c5b0
Merge pull request #37 from mosharaf13/release/0.2.0
mosharaf13 May 30, 2023
f095c0a
Upload file
May 30, 2023
1065845
Insert to database
May 30, 2023
5f732a9
Add Bootstrap classes
May 30, 2023
46d87d2
Add validation
May 30, 2023
70b5a14
Add helpers
May 30, 2023
b48ad3b
Add fixture files
May 31, 2023
b974df7
Update controller
May 31, 2023
fbd3d54
Add tests
May 31, 2023
f3a5ea0
Accept only CSV
May 31, 2023
d51d977
Add file validations
May 31, 2023
f7349ba
Update fixture files
May 31, 2023
fb1689f
Validate maximum keywords
May 31, 2023
035589f
Update fixture files
May 31, 2023
6ec2e4c
Update tests
May 31, 2023
3edef7e
Add support file
May 31, 2023
2004806
Remove manual file size check
May 31, 2023
56b8585
Update tests
May 31, 2023
db940aa
Update path
May 31, 2023
9fb4021
Update fixture files
Jun 1, 2023
526182b
Update redirection
Jun 1, 2023
8241db7
Fix tests
Jun 1, 2023
e8851a2
Merge remote-tracking branch 'origin/backend/list-keywords' into feat…
Jun 1, 2023
565303c
Update tests
Jun 1, 2023
2ce0def
Update code
Jun 1, 2023
4925fd2
Update tests
Jun 1, 2023
93d1548
Update code
Jun 1, 2023
9c29c04
Update seeds
Jun 1, 2023
4895f2e
Insert current user ID
Jun 1, 2023
215cc21
Remove validation
Jun 1, 2023
91e5b47
Update style
Jun 1, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions app/controllers/search_stats_controller.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,38 @@
# frozen_string_literal: true

require 'csv'

class SearchStatsController < ApplicationController
ALLOWED_MIME_TYPE = 'text/csv'
MAXIMUM_KEYWORDS = 1000

# GET /search_stats
def index
@pagy, @search_stats = pagy(SearchStat.all)
end

def new
search_stat = SearchStat.new

render :new, locals: { search_stat: search_stat }
end

def create
Copy link

@github-actions github-actions bot Jun 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

@github-actions github-actions bot Jun 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@longnd longnd Jun 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

@github-actions github-actions bot Jun 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Has approx 10 statements

csv_file = params[:csv_file]
Copy link

@github-actions github-actions bot Jun 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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

csv_file_content = params[:csv_file].read
Copy link

@github-actions github-actions bot Jun 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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
Comment on lines +21 to +29
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


keywords.map do |keyword|
search_stat = SearchStat.new({ keyword: keyword, user_id: current_user.id })
search_stat.save!
end

redirect_to search_stats_path
end
end
1 change: 0 additions & 1 deletion app/models/search_stat.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,4 @@

class SearchStat < ApplicationRecord
validates :keyword, presence: true
validates :raw_response, presence: true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

end
4 changes: 4 additions & 0 deletions app/views/search_stats/new.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<%= form_with url: '/search_stats', multipart: true do |form| %>
<%= file_field_tag :csv_file, accept: 'text/csv', class: 'form-control' %>
<%= form.submit 'Upload', class: 'btn btn-primary mt-3' %>
<% end %>
2 changes: 1 addition & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@
root "home#index"

get "/health_check", to: 'health_check#health_check', as: :rails_health_check
resources :search_stats, only: [:index]
resources :search_stats, only: [:index, :create, :new]
end
2 changes: 2 additions & 0 deletions db/seeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

require 'fabrication'

Fabricate(:user, email: 'demo@example.com', password: 'nimbleHq1234')

# Generate dummy data for SearchStat
10.times do
Fabricate(:search_stat)
Expand Down
3 changes: 3 additions & 0 deletions spec/fixtures/files/invalid_data.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@



3 changes: 3 additions & 0 deletions spec/fixtures/files/invalid_type.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
1st keyword
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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')

2nd keyword
3rd keyword
3 changes: 3 additions & 0 deletions spec/fixtures/files/keywords.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
1st keyword
2nd keyword
3rd keyword
Loading