diff --git a/app/assets/stylesheets/common_elements.css b/app/assets/stylesheets/common_elements.css index 04077a52..7386c731 100644 --- a/app/assets/stylesheets/common_elements.css +++ b/app/assets/stylesheets/common_elements.css @@ -1,5 +1,6 @@ a, .link { + @apply cursor-pointer; @apply text-pink-700; @apply rounded; &:hover { diff --git a/app/assets/stylesheets/editable.css b/app/assets/stylesheets/editable.css index 59976dc6..f0b56080 100644 --- a/app/assets/stylesheets/editable.css +++ b/app/assets/stylesheets/editable.css @@ -8,15 +8,19 @@ } .edit-button { @apply rounded-lg py-2 px-3; + &.edit-button--small { @apply rounded-lg py-1 px-1.5; @apply text-sm; } + @apply bg-gray-100 border border-gray-100; @apply inline-flex gap-2 content-center items-center; @apply text-gray-700; + &.collapsable { @apply gap-0; + .text { max-width: 0; overflow: hidden; @@ -28,11 +32,18 @@ &:hover { @apply shadow-md; @apply border border-pink-700; - &.collapsable { - .text { - max-width: 200px; - margin-right: 0.5em; - } - } } } + +.collapsable-wrapper:hover .collapsable, +.edit-button.collapsable:hover { + .text { + max-width: 200px; + margin-right: 0.5em; + } +} + +.collapsable-wrapper:hover .edit-button { + @apply shadow-md; + @apply border border-pink-700; +} diff --git a/app/controllers/facility_reviews_controller.rb b/app/controllers/facility_reviews_controller.rb index c600f168..3d7da449 100644 --- a/app/controllers/facility_reviews_controller.rb +++ b/app/controllers/facility_reviews_controller.rb @@ -1,20 +1,18 @@ # frozen_string_literal: true class FacilityReviewsController < BaseControllers::AuthenticateController - def new - @space = Space.find(params["space_id"]) - @categories = FacilityCategory.all + include DefineGroupedFacilitiesForSpace - @reviews_for_categories = @space.reviews_for_categories(current_user) + def new + params.require([:space_id, :facility_id, :facility_category_id]) - @grouped_relevant_facilities = @space.relevant_space_facilities(grouped: true) - @non_relevant_facilities = @space.non_relevant_space_facilities - @grouped_non_relevant_facilities = @space.group_space_facilities(@non_relevant_facilities) + @space = Space.find(params[:space_id]) + @facility = Facility.find(params[:facility_id]) + @space_facility = SpaceFacility.find_by(space: @space, facility: @facility) + @facility_review = FacilityReview.find_or_initialize_by(facility: @facility, space: @space, user: current_user) + @experiences = FacilityReview::LIST_EXPERIENCES - @experiences = [ - "unknown", - *FacilityReview.experiences.keys.reverse - ].reverse + define_category end def create @@ -54,7 +52,8 @@ def redirect_and_show_flash(flash_message:, flash_type: :notice, redirect_status respond_to do |format| format.turbo_stream do flash.now[flash_type] = flash_message - @grouped_relevant_facilities = @space.relevant_space_facilities(grouped: true) + + define_facilities render turbo_stream: [ turbo_stream.update(:flash, partial: "shared/flash"), @@ -133,4 +132,12 @@ def nothing_changed(review, existing_review) existing_review.present? && existing_review[:experience] == review["experience"] end + + def define_category + @category = if params[:facility_category_id].present? + FacilityCategory.find(params[:facility_category_id]) + else + Facility.find(params[:facility_id]).facility_categories.first + end + end end diff --git a/app/controllers/spaces_controller.rb b/app/controllers/spaces_controller.rb index 9365da7f..58202924 100644 --- a/app/controllers/spaces_controller.rb +++ b/app/controllers/spaces_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class SpacesController < BaseControllers::AuthenticateController # rubocop:disable Metrics/ClassLength + include DefineGroupedFacilitiesForSpace + def index @spaces = Space.order updated_at: :desc @space = Space.new @@ -9,7 +11,8 @@ def index def show @space = Space.includes(:space_contacts).where(id: params[:id]).first @space_contact = SpaceContact.new(space_id: @space.id, space_group_id: @space.space_group_id) - @grouped_relevant_facilities = @space.relevant_space_facilities(grouped: true) + + define_facilities end def new diff --git a/app/models/facility.rb b/app/models/facility.rb index 4352ad97..d18b172f 100644 --- a/app/models/facility.rb +++ b/app/models/facility.rb @@ -3,6 +3,7 @@ class Facility < ApplicationRecord has_many :facilities_categories, dependent: :destroy has_many :facility_categories, through: :facilities_categories + has_many :facility_reviews, dependent: :destroy has_many :space_types_facilities, dependent: :destroy has_many :space_types, through: :space_types_facilities diff --git a/app/models/facility_review.rb b/app/models/facility_review.rb index 46a97047..e71fab49 100644 --- a/app/models/facility_review.rb +++ b/app/models/facility_review.rb @@ -17,6 +17,11 @@ def experience_icon ICON_FOR_EXPERIENCE[experience] end + LIST_EXPERIENCES = [ + "unknown", + *FacilityReview.experiences.keys.reverse + ].reverse + ICON_FOR_EXPERIENCE = { "was_allowed" => "likely", "was_not_allowed" => "unlikely", diff --git a/app/models/space.rb b/app/models/space.rb index 8f229fa8..d400fd28 100644 --- a/app/models/space.rb +++ b/app/models/space.rb @@ -152,74 +152,93 @@ def score_from_star_rating (star_rating - 2.9) / 10 end - # Groups all of the users facility reviews into a hash like - # { category_id: { facility_id: review } } - def reviews_for_categories(user) - user.facility_reviews - .where(space: self) - .includes(facility: [:facilities_categories]) - .joins(facility: [:facilities_categories]) - .each_with_object({}) do |review, memo| - review.facility.facilities_categories.each do |facility_category| - memo[facility_category.facility_category_id] ||= {} - memo[facility_category.facility_category_id][review.facility.id] = review - end - end - end - # Space Facilities that are typically relevant for the space # Either because they share a space type with the space # or because someone has said that they are relevant for # this specific space by setting a space_facilities experience. # - # Can be grouped by category by passing grouped: true - def relevant_space_facilities(grouped: false) - relevant = space_facilities.includes(facility: [:facilities_categories]).where(relevant: true) - - return relevant unless grouped - - group_space_facilities(relevant) - end - - # Facilities (found through space facilities) that are relevant. - def relevant_facilities - space_facilities.includes(:facility).where(relevant: true).map(&:facility) + # Can be grouped by category by passing grouped: true, and + # if a user is defined, include facility reviews for that user + def relevant_space_facilities(grouped: false, user: nil) + filtered_space_facilities(relevant: true, grouped:, user:) end # Space Facilities that are typically NOT relevant for the space # Because they DO NOT share a space type with the space AND # no one has given a space_facility experience for them. # - # Can be grouped by category by passing grouped: true + # Can be grouped by category by passing grouped: true. + # Cannot have facility reviews, else they would be relevant. def non_relevant_space_facilities(grouped: false) - non_relevant = space_facilities.includes(facility: [:facilities_categories]).where(relevant: false) + filtered_space_facilities(relevant: false, grouped:) + end - return non_relevant unless grouped + def filtered_space_facilities(relevant: true, grouped: false, user: nil) + ungrouped_facilities = space_facilities + .includes( + facility: [ + :facility_reviews, + :facilities_categories + ] + ) + .where(relevant:) + .order("facilities.title") - group_space_facilities(non_relevant) + return ungrouped_facilities unless grouped + + group_space_facilities(ungrouped_facilities:, user:) + end + + # Facilities (found through space facilities) that are relevant. + def relevant_facilities + space_facilities.includes(:facility).where(relevant: true).map(&:facility) end # Groups given facilities by their category # { category_id: [facility_1, facility_2, ...] } - def group_space_facilities(ungrouped_facilities) # rubocop:disable Metrics/AbcSize + def group_space_facilities(ungrouped_facilities:, user: nil) ungrouped_facilities.each_with_object({}) do |space_facility, memo| space_facility.facility.facilities_categories.each do |facility_category| memo[facility_category.facility_category_id] ||= { category: facility_category.facility_category, space_facilities: [] } - memo[facility_category.facility_category_id][:space_facilities] << { - id: space_facility.facility.id, - title: space_facility.facility.title, - description: space_facility.description, - review: space_facility.experience, - space_types: space_facility.facility.space_types, - relevant: space_facility.relevant - } + + facility_data = facility_data_from_space_facility(space_facility:, facility_category:, user:) + + memo[facility_category.facility_category_id][:space_facilities] << facility_data end end.sort_by(&:first) # sorts by category id end + def facility_data_from_space_facility(space_facility:, facility_category:, user:) + data = { + id: space_facility.facility.id, + title: space_facility.facility.title, + description: space_facility.description, + review: space_facility.experience, + space_types: space_facility.facility.space_types, + relevant: space_facility.relevant, + category_id: facility_category.facility_category.id + } + + add_current_user_review_to_data(data:, space_facility:, user:) + end + + def add_current_user_review_to_data(data:, space_facility:, user:) + return data if user.blank? + + if space_facility.facility.facility_reviews.present? + current_user_review = space_facility.facility.facility_reviews.find_by(user:, + space: self) + end + + data[:current_user_review] = + (current_user_review.presence || FacilityReview.new(facility_id: id, space_id: @space, user_id: user.id)) + + data + end + def merge_paper_trail_versions PaperTrail::Version .or(PaperTrail::Version.where(item: self)) diff --git a/app/models/space_facility.rb b/app/models/space_facility.rb index 1acb9d75..b17f6481 100644 --- a/app/models/space_facility.rb +++ b/app/models/space_facility.rb @@ -26,6 +26,14 @@ def not_relevant! def not_relevant? !relevant? end + + def user_review(user) + FacilityReview.find_or_initialize_by( + user:, + facility:, + space: + ) + end end # == Schema Information diff --git a/app/models/user.rb b/app/models/user.rb index aa88ef04..ba813b3f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -21,8 +21,8 @@ def name "#{first_name} #{last_name[0]&.upcase}." end - def new_facility_review_for(facility_id, space) - FacilityReview.new( + def facility_review_for(facility_id, space) + FacilityReview.find_or_initialize_by( space:, facility_id:, user: self diff --git a/app/services/spaces/aggregate_facility_reviews_service.rb b/app/services/spaces/aggregate_facility_reviews_service.rb index e90e5994..3caa5674 100644 --- a/app/services/spaces/aggregate_facility_reviews_service.rb +++ b/app/services/spaces/aggregate_facility_reviews_service.rb @@ -36,9 +36,10 @@ def aggregate_reviews(facility) # rubocop:disable Metrics/AbcSize return handle_zero_facility_reviews(space_facility, belongs_to_space_type) if count.zero? # Set criteria: - positive_threshold = reviews.positive.count >= (count / 3.0 * 2.0).ceil - impossible_threshold = reviews.impossible.count >= (count / 2.0).ceil - negative_threshold = reviews.negative.count >= (count / 3.0 * 2.0).ceil + two_out_of_three = (count / 3.0 * 2.0).ceil + positive_threshold = reviews.positive.count >= two_out_of_three + impossible_threshold = reviews.impossible.count >= two_out_of_three + negative_threshold = reviews.negative.count >= two_out_of_three set_relevance(space_facility, belongs_to_space_type, positive_threshold) diff --git a/app/views/facility_reviews/_facility_with_edit_button.html.erb b/app/views/facility_reviews/_facility_with_edit_button.html.erb new file mode 100644 index 00000000..b3cd3874 --- /dev/null +++ b/app/views/facility_reviews/_facility_with_edit_button.html.erb @@ -0,0 +1,28 @@ +<%# locals: (title:, tooltip:, space_id:, facility_review:, experience:, facility_category_id:, facility_id:, description: %> +<%= turbo_frame_tag dom_id(facility_review, "category_#{facility_category_id}__facility_#{facility_id}__") do %> + <%= link_to new_facility_review_path( + space_id:, + facility_id:, + facility_category_id:, + ), + title: tooltip, + class: 'unstyled-link inline-flex gap-2 items-center collapsable-wrapper' do %> + +

"><%= title %>

+ + <%= inline_svg_tag "facility_status/#{experience}", class: "#{" text-black " if experience == "impossible" }" %> + + + + <%= t('space_edit.edit') %> + + <%= inline_svg_tag 'edit', alt: t('space_edit.edit'), title: t('space_edit.edit') %> + + <% end %> + + <% unless description.nil? %> +
+ <%= description %> +
+ <% end %> +<% end %> diff --git a/app/views/facility_reviews/new.html.erb b/app/views/facility_reviews/new.html.erb index 80cb918e..3406b6a1 100644 --- a/app/views/facility_reviews/new.html.erb +++ b/app/views/facility_reviews/new.html.erb @@ -1,60 +1,89 @@ -<%= turbo_frame_tag :new_facility_review_path do %> - <%= simple_form_for @space, - url: create_facility_review_path(@space), - method: :post, - data: { - controller: 'sync-fields-with-same-id', - 'sync-fields-with-same-id-target': 'form' - } do |form| %> - - - + <% end %> -<% end %> + diff --git a/app/views/facility_reviews/new/_facility_buttons.html.erb b/app/views/facility_reviews/new/_facility_buttons.html.erb deleted file mode 100644 index 5d1a518b..00000000 --- a/app/views/facility_reviews/new/_facility_buttons.html.erb +++ /dev/null @@ -1,95 +0,0 @@ -<%# -Locals: -- facility (one Facility) -- form (Form for a new or editing FacilityReviews) -- facility_review (a new or old facility) -%> - -
  • - <%= form.simple_fields_for :facility_reviews, - facility_review, - # Child index is used to make sure that the form fields are - # named after the facility id, so duplicate facility fields can be - # found and synced with sync_fields_with_same_id.js - child_index: facility[:id] do |f| %> - <%= f.hidden_field :facility_id, value: facility[:id] %> - <%= f.hidden_field :user_id, value: current_user.id %> - <%= f.hidden_field :id, value: facility_review.id %> -
    - -
    -

    "><%= facility[:title] %>

    - - - <%= inline_svg_tag "facility_status/#{facility[:review]}", class: "#{" text-black " if facility[:review] == "impossible" }" %> - <%= t("tooltips.others_experience.#{facility[:review]}") %> - - - - -
    -
    - - <% if facility[:description] %> -
    - <%= facility[:description] %> -
    - <% end %> - - -
    - <% end %> -
  • diff --git a/app/views/facility_reviews/new/_facility_category.html.erb b/app/views/facility_reviews/new/_facility_category.html.erb deleted file mode 100644 index 9f492292..00000000 --- a/app/views/facility_reviews/new/_facility_category.html.erb +++ /dev/null @@ -1,14 +0,0 @@ -<% if facilities_in_category.present? %> - <%= tag.h3 category.title %> - -
    -<% end %> diff --git a/app/views/spaces/index/_facility_item.html.erb b/app/views/spaces/index/_facility_item.html.erb new file mode 100644 index 00000000..c9adf181 --- /dev/null +++ b/app/views/spaces/index/_facility_item.html.erb @@ -0,0 +1,15 @@ +
  • + "> + <%= title %> + + <%= inline_svg_tag "facility_status/#{review}" %> + + + <% unless description.nil? %> +
    + <%= description %> +
    + <% end %> + +
  • diff --git a/app/views/spaces/index/_space_facility_listing.html.erb b/app/views/spaces/index/_space_facility_listing.html.erb index 9bec3ada..bbe4da7a 100644 --- a/app/views/spaces/index/_space_facility_listing.html.erb +++ b/app/views/spaces/index/_space_facility_listing.html.erb @@ -2,7 +2,7 @@ <% filtered_facilities.each do |facility| %> <% next unless space.relevance_of_facility(facility) %> <% review = space.reviews_for_facility(facility) %> - <%= render partial: 'spaces/show/facility_item', locals: { + <%= render partial: 'spaces/index/facility_item', locals: { title: facility[:title], review: review, description: nil, diff --git a/app/views/spaces/show/_facilities.html.erb b/app/views/spaces/show/_facilities.html.erb index 0ee9eb98..6f56ed43 100644 --- a/app/views/spaces/show/_facilities.html.erb +++ b/app/views/spaces/show/_facilities.html.erb @@ -2,45 +2,30 @@

    <%= Facility.model_name.human(count: 2) %>

    - <%= button_to_modal :new_facility_review_path, - "aria-label": "Rediger fasiliteter", - class: 'unstyled-link edit-button collapsable' do %> - - Rediger - - <%= inline_svg_tag 'edit', alt: 'Redigér', title: 'Redigér' %> - <% end %>
    <% if @grouped_relevant_facilities.present? %>
    <% @grouped_relevant_facilities.each do | group | %> -
    -

    - <%= group.second[:category].title %> -

    - - -
    + <%= render 'spaces/show/facility_group', group: %> <% end %>
    <% end %> - + <% if @grouped_non_relevant_facilities.present? %> +

    + <%= t('facility_reviews.new.showing_relevant_facilities', + space_title: @space.title, + space_types: @space.space_types.map{ |type| type.type_name }.to_sentence) %> +

    +
    + + Vis <%= @non_relevant_facilities.count %> andre fasiliteter + + <% @grouped_non_relevant_facilities.each do |group| %> + <%= render 'spaces/show/facility_group', group: %> + <% end %> +
    + <% end %> diff --git a/app/views/spaces/show/_facility_group.html.erb b/app/views/spaces/show/_facility_group.html.erb new file mode 100644 index 00000000..8f4c9436 --- /dev/null +++ b/app/views/spaces/show/_facility_group.html.erb @@ -0,0 +1,14 @@ +
    +

    + <%= group.second[:category].title %> +

    + + +
    diff --git a/app/views/spaces/show/_facility_item.html.erb b/app/views/spaces/show/_facility_item.html.erb index c9adf181..8a156703 100644 --- a/app/views/spaces/show/_facility_item.html.erb +++ b/app/views/spaces/show/_facility_item.html.erb @@ -1,15 +1,15 @@ -
  • - "> - <%= title %> - - <%= inline_svg_tag "facility_status/#{review}" %> - - - <% unless description.nil? %> -
    - <%= description %> -
    - <% end %> - +<%# locals: (facility:, space:) %> +
  • + <%= render partial: 'facility_reviews/facility_with_edit_button', + locals: { + facility_id: facility[:id], + title: facility[:title], + experience: facility[:review], + description: facility[:description], + facility_category_id: facility[:category_id], + facility_review: facility[:current_user_review], + tooltip: t("tooltips.facility_aggregated_experience.#{facility[:review]}"), + space_id: space[:id] + } + %>
  • diff --git a/config/locales/nb.yml b/config/locales/nb.yml index 54ec3169..b19c23da 100644 --- a/config/locales/nb.yml +++ b/config/locales/nb.yml @@ -109,7 +109,7 @@ nb: back_to_space: "Tilbake til %{title}" facility_reviews: new: - showing_relevant_facilities: 'Viser kun fasiliteter som er spesielle for %{space_title} eller vanlige for lokaler av typen "%{space_types}".' + showing_relevant_facilities: 'Viser kun fasiliteter med erfaringer fra %{space_title} eller som er vanlige for %{space_types}.' show_other_facilities: one: 'Vis den siste fasiliteten' other: 'Vis %{count} andre fasiliteter' diff --git a/config/routes.rb b/config/routes.rb index 65ef6ed1..2db6fb62 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -30,7 +30,8 @@ resources "space_groups", except: "new" resources "space_contacts", only: [:create, :edit, :update, :destroy, :show] - get "/spaces/:space_id/facility_review", to: "facility_reviews#new", as: "new_facility_review" + get "/spaces/:space_id/facility_review/:facility_id/:facility_category_id", to: "facility_reviews#new", + as: "new_facility_review" post "/spaces/:space_id/facility_review", to: "facility_reviews#create", as: "create_facility_review" # Review routes diff --git a/lib/define_grouped_facilities_for_space.rb b/lib/define_grouped_facilities_for_space.rb new file mode 100644 index 00000000..50635c40 --- /dev/null +++ b/lib/define_grouped_facilities_for_space.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module DefineGroupedFacilitiesForSpace + private + + def define_facilities + @grouped_relevant_facilities = @space.relevant_space_facilities(grouped: true, user: current_user) + @non_relevant_facilities = @space.non_relevant_space_facilities + @grouped_non_relevant_facilities = @space.group_space_facilities(ungrouped_facilities: @non_relevant_facilities, + user: current_user) + @experiences = FacilityReview::LIST_EXPERIENCES + end +end diff --git a/spec/support/user_creates_facility_reviews_helpers.rb b/spec/support/user_creates_facility_reviews_helpers.rb index 8fa90ae9..2c601f31 100644 --- a/spec/support/user_creates_facility_reviews_helpers.rb +++ b/spec/support/user_creates_facility_reviews_helpers.rb @@ -69,44 +69,34 @@ def expect_to_add_multiple_facility_reviews( def click_to_create_new_facility_review(facility:, description_to_add: "") - open_form_modal enter_data_about_facility(facility:, description_to_add:) - - save_form_modal end def click_to_create_multiple_facility_reviews(facilities_to_review: []) - open_form_modal - facilities_to_review.each do |facility| enter_data_about_facility(facility:) end - - save_form_modal - end - - def open_form_modal - # Click the edit button - find("button[aria-label='Rediger fasiliteter']").click - end - - def save_form_modal - click_on(I18n.t("facility_reviews.save")) - - # Wait for the save to go through - expect(page).to have_text(I18n.t("reviews.added_review")) end def enter_data_about_facility(facility:, description_to_add: "") - fieldset = first("fieldset", text: facility.title) - within(fieldset) do - click_on("Rediger") + edit_button = first("a", text: facility.title) + edit_button.click + + form = find("form", text: facility.title, wait: 10) + within(form) do choose(I18n.t("reviews.form.facility_experience_particular_tense.was_allowed"), allow_label_click: true) # Add a description fill_in(I18n.t("simple_form.labels.space_facility.description"), with: description_to_add) + + # And save + submit_button = find("button[type=submit]") + submit_button.click end + + # Then wait until the save is done and flash message is shown + expect(page).to have_text(I18n.t("reviews.added_review")) end # rubocop:disable Metrics/ParameterLists, Metrics/MethodLength, Metrics/AbcSize # This is a test helper, it's fine @@ -131,8 +121,8 @@ def expect_changes_around_reviewing_facility( count_of_visible_facilities - count_should_have_new_experience # CSS selectors for reviewed and non reviewed facilities - no_reviews_selector = "li[title='#{I18n.t('tooltips.facility_aggregated_experience.unknown')}']" - reviewed_selector = "li[title='#{I18n.t("tooltips.facility_aggregated_experience.#{expected_new_experience}")}']" + no_reviews_selector = "a[title='#{I18n.t('tooltips.facility_aggregated_experience.unknown')}']" + reviewed_selector = "a[title='#{I18n.t("tooltips.facility_aggregated_experience.#{expected_new_experience}")}']" relevant_space_facilities = space.relevant_space_facilities space_facilities_matching_facility = relevant_space_facilities.where(facility:) @@ -163,7 +153,7 @@ def expect_changes_around_reviewing_facility( # Check that the reviewed facility changed in the model: expect( space_facilities_matching_facility.reload.all? do |space_facility| - space_facility.experience == expected_new_experience + space_facility.reload.experience == expected_new_experience end ).to be(true)