Follow-up from "Milestones finder - filter by id OR title"
The following discussion from !139809 (merged) should be addressed:
-
@mhamda started a discussion: (+2 comments) thought (non-blocking): I think we can improve readability here if we use methods, eg.
def by_ids_or_title(items) return items if both_params_blank? return items.id_in(params[:ids]) if only_ids_present? return items.with_title(params[:title]) if only_title_present? items.with_ids_or_title(ids: params[:ids], title: params[:title]) end private def both_params_blank? params[:ids].blank? && params[:title].blank? end def only_ids_present? params[:ids].present? && params[:title].blank? end def only_title_present? params[:ids].blank? && params[:title].present? end
We can also adjust the scope
scope :with_ids_or_title, ->(ids:, title:) { id_in(ids).or(with_title(title)) }
To handle both cases?scope :with_ids_or_title, ->(ids:, title:) { result = all # if we want to remove `Milestone.all` https://gitlab.com/gitlab-org/gitlab/-/merge_requests/139809/diffs#4fec946d103b277a8ae7b9d1e702a488cdb46e3f_29_29 result = result.id_in(ids) if ids.present? result = result.with_title(title) if title.present? result }
Usage:
def by_ids_or_title(items) items.with_ids_or_title(ids: params[:ids], title: params[:title]) end
All of this can be a follow-up if you agree
🙏🏼