Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Api::V1::SearchResultsController should use strong params #7

Open
olivierobert opened this issue Apr 23, 2018 · 1 comment
Open

Api::V1::SearchResultsController should use strong params #7

olivierobert opened this issue Apr 23, 2018 · 1 comment

Comments

@olivierobert
Copy link

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
@ankitkalia1195
Copy link
Owner

SearchTasks controller has create action which is using strong parameters(so it is absolute necessity)...For index action and search it might or might not be required depending on the situtation, we can also use ransackable attributes feature of ransack to allow ransack for only particular attributes. Altough i agree minor code refractoring can be done to make this action look more readable like using before_action :modify_params.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants