searchbot's People
searchbot's Issues
SearchTaskJob Improvements
-
Not sure about the need to define
process!
in the model itself. Since it will only be used in the job, it could be defined there. -
Use a transaction block to revert database changes in case of errors
-
Never rescue from
Exception
as it's the parent class of all errors, hence it could catch errors you don't want to catch -
Use custom errors to map errors the app domain
-
Use Active Job hooks to perform actions after successful job completions
class ProcessSearchTaskJob < ApplicationJob
def after_perform do |job|
search_task.update_column(:state, :complete)
end
def perform(search_task)
ActiveRecord::Base.transaction do
search_task.update_column(:state, :processing)
parse_keywords_and_create_reports
end
rescue StandardError => exception
raise ActiveRecord::Rollback
search_task.update_column(:state, :failed)
raise Searchbot::Errors::ProcessKeywordError.new
end
end
Missing README details
README should contain the basic details about the project, how to set it up and test.
Test fixtures should be stored in spec/fixture
Many tests are hard to read because the fixtures are placed in the spec files.
Bacseu fixtures take loads of space, it's better to store and group all file-based fixtures inside a sub-directory.
RSpec.describe SearchReport do
let(:search_report) { create(:search_report) }
let(:google_search_parser) { GoogleSearchParser.new('new york tours') }
let(:html_source) { File.read(Rails.root.join('spec', 'fixtures', 'google-result-page.html')) }
describe 'Associations' do
it { is_expected.to belong_to(:search_task) }
it { is_expected.to have_many(:search_results).dependent(:destroy) }
end
[...]
end
Missing tests
Upon review of the implementation, the overall architecture seems sound but there are not unit or UI tests to back it up.
You are free to choose between MiniTest or RSpec but tests are required at the very least for:
- Models
- Jobs
- Services
- Systems (UI) specs for the main flow to upload and see search results
Api::V1::SearchResultsController should use strong params
While Api::V1::SearchTasksController
uses strong params: params.require(:search_task).permit(:name, :keywords_csv)
, the other APi controller does not.
In addition the params naming is a kind of cryptic:
params[:q]
Instead, it could be named fully as query
.
In the params, this following is kind of hard to understand:
params[:q][:search_report_search_task_user_id_eq] = current_resource_owner.id
I am guessing this is required by ransack
but it's a bit awkward and undocumented.
A better way would be to move this to a Service Class to remove this complexity from the controller. If this does not makes sense, minor improvements like this could help:
class SearchResultsController < BaseController
def index
search_result = SearchResult.ransack(search_params).result.includes(search_report: :search_task)
[...]
render json: search_tasks, load_associations: true, search_report_ids: search_report_ids,
earch_result_ids: search_result_ids, include: ['search_reports', 'search_results']
end
private
def search_params
params.require(:query)
append_ransack_params
end
def append_ransack_params
params.merge(search_report_search_task_user_id_eq: current_resource_owner.id)
end
end
Why not defining the current_resource_owner in doorkeeper.rb?
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. ๐๐๐
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google โค๏ธ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.