From 84bca4741036778801032c47e2363d853f66648e Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Fri, 16 Feb 2024 21:25:20 +0100 Subject: [PATCH] Clean up facility reviews controller --- .../facility_reviews_controller.rb | 99 ++++++++++++------- 1 file changed, 61 insertions(+), 38 deletions(-) diff --git a/app/controllers/facility_reviews_controller.rb b/app/controllers/facility_reviews_controller.rb index 039fed31..c600f168 100644 --- a/app/controllers/facility_reviews_controller.rb +++ b/app/controllers/facility_reviews_controller.rb @@ -29,33 +29,31 @@ def create @affected_facilities = [] # filtered_facility_reviews will mutate this array relevant_reviews = filtered_facility_reviews(reviews_and_descriptions[:facility_reviews_attributes].values) - return render :new, status: :unprocessable_entity unless FacilityReview.create!(relevant_reviews) + return create_failed unless FacilityReview.create!(relevant_reviews) - # NB: Under the current model, this needs to happen before we add the descriptions, - # as aggregating DELETES all space_facilities + # NB: Under the current model, this needs to happen before we add the descriptions with update_space_facilities, + # as aggregating DESTROYS all space_facilities, including the descriptions (wtf) @space.aggregate_facility_reviews(facilities: @affected_facilities) - update_space_facilities(reviews_and_descriptions[:space_facilities_attributes].values) - create_success + create_succeeded end private - def update_space_facilities(space_facilities) - space_facilities.each do |facility| - next if facility[:description].blank? + def create_failed + redirect_and_show_flash(flash_message: t("reviews.error"), flash_type: :error, + redirect_status: :unprocessable_entity) + end - SpaceFacility.find_by(facility_id: facility[:facility_id], space: @space.id) - .update!(description: facility[:description]) - end + def create_succeeded + redirect_and_show_flash(flash_message: t("reviews.added_review")) end - def create_success - flash_message = t("reviews.added_review") + def redirect_and_show_flash(flash_message:, flash_type: :notice, redirect_status: :ok) respond_to do |format| format.turbo_stream do - flash.now[:notice] = flash_message + flash.now[flash_type] = flash_message @grouped_relevant_facilities = @space.relevant_space_facilities(grouped: true) render turbo_stream: [ turbo_stream.update(:flash, @@ -65,49 +63,74 @@ def create_success ] end format.html do - flash[:notice] = flash_message - redirect_to @space + flash[flash_type] = flash_message + redirect_to @space, status: redirect_status end end end + def update_space_facilities(space_facilities) + space_facilities.each do |facility| + next if facility[:description].blank? + + SpaceFacility.find_by(facility_id: facility[:facility_id], space: @space.id) + .update!(description: facility[:description]) + end + end + # Does two things: # 1. Destroys any existing reviews that are changed, and returns an array of new reviews to be created # 2. And mutates affected_facilities so we can aggregate facilities for those - def filtered_facility_reviews(facility_reviews) # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity + def filtered_facility_reviews(facility_reviews) facility_reviews.filter_map do |review| - # We minimum need these to do anything useful - unless review[:experience].present? && - review[:user_id].present? && - review[:facility_id].present? && - @space.id.present? - next - end + next unless facility_review_has_required_params?(review) - existing_review = FacilityReview.find_by(user_id: review[:user_id], - facility_id: review[:facility_id], - space_id: @space.id) + # Check if something is new or changed + existing_review = find_existing_facility_review(review) + next if nothing_changed(review, existing_review) - # NB: Unknown experiences do not create reviews. Thus existing_review.blank? is the same as - # experience == "unknown" - nothing_changed = (existing_review.blank? && review["experience"] == "unknown") || - (existing_review.present? && existing_review[:experience] == review["experience"]) - - next if nothing_changed + # Destroy any old review, so we are ready to return a new review for creation. + existing_review.destroy if existing_review.present? # Something changed, so add facility to affected facilities @affected_facilities << Facility.find(review["facility_id"]) - # Destroy the old review, so we are ready to return a new review for creation. - existing_review.destroy if existing_review.present? - - # Unknown experiences do not create reviews. + # Unknown experiences do not create reviews, so we can just skip creating a new one: next if review["experience"] == "unknown" - # Add space_id to the review + # Space id is not in the form, we add it from params instead review["space_id"] = @space.id + # New Facility Review, ready to be made! review end end + + def facility_review_has_required_params?(facility_review) + # We minimum need these to do anything useful + facility_review[:experience].present? && + facility_review[:user_id].present? && + facility_review[:facility_id].present? && + @space.id.present? + end + + def find_existing_facility_review(review) + existing_review = FacilityReview.find_by(id: review[:id]) if review[:id].present? + return existing_review if existing_review.present? + + FacilityReview.find_by( + facility_id: review[:facility_id], + space_id: @space.id, + user_id: review[:user_id] + ) + end + + def nothing_changed(review, existing_review) + # NB: Unknown experiences do not create reviews. + # Thus existing_review.blank? is the same as + # experience == "unknown" + return true if existing_review.blank? && review["experience"] == "unknown" + + existing_review.present? && existing_review[:experience] == review["experience"] + end end