Merge pull request #4294 from consul/sdg_revisions

Extend SDG managment section to allow content reviews

.rubocop.yml

@@ -360,7 +360,6 @@ RSpec/HookArgument:
 RSpec/InstanceVariable:
   Enabled: true
   Exclude:
-    - spec/controllers/concerns/has_filters_spec.rb
     - spec/controllers/concerns/has_orders_spec.rb
 
 RSpec/LetBeforeExamples:

app/components/sdg_management/relations/index_component.html.erb

@@ -7,8 +7,11 @@
   <%= component.select_tag :target_code, target_options,
                            include_blank: target_blank_option,
                            "aria-label": target_label %>
+  <%= component.hidden_field_tag :filter, current_filter %>
 <% end %>
 
+<%= render "shared/filter_subnav", i18n_namespace: "sdg_management.relations.index" %>
+
 <table>
   <thead>
     <tr>

app/components/sdg_management/relations/index_component.rb

@@ -1,6 +1,7 @@
 class SDGManagement::Relations::IndexComponent < ApplicationComponent
   include Header
   include SDG::Goals::OptionsForSelect
+  delegate :valid_filters, :current_filter, to: :helpers
 
   attr_reader :records
 

app/components/sdg_management/subnavigation_component.rb

@@ -12,9 +12,11 @@ class SDGManagement::SubnavigationComponent < ApplicationComponent
     end
 
     def link_to_section(section)
-      link_to "SDG::#{section.to_s.classify}".constantize.model_name.human(count: 2).titleize,
-              path_for(section),
-              class: active_style(section)
+      link_to text_for(section), path_for(section), class: active_style(section)
+    end
+
+    def text_for(section)
+      "SDG::#{section.to_s.classify}".constantize.model_name.human(count: 2).titleize
     end
 
     def path_for(section)

app/controllers/concerns/has_filters.rb

@@ -1,5 +1,10 @@
 module HasFilters
   extend ActiveSupport::Concern
+  attr_reader :valid_filters, :current_filter
+
+  included do
+    helper_method :valid_filters, :current_filter
+  end
 
   class_methods do
     def has_filters(valid_filters, *args)

app/controllers/sdg_management/relations_controller.rb

@@ -2,8 +2,12 @@ class SDGManagement::RelationsController < SDGManagement::BaseController
   before_action :check_feature_flags
   before_action :load_record, only: [:edit, :update]
 
+  FILTERS = %w[pending_sdg_review all sdg_reviewed].freeze
+  has_filters FILTERS, only: :index
+
   def index
     @records = relatable_class
+               .send(@current_filter)
                .accessible_by(current_ability)
                .by_goal(params[:goal_code])
                .by_target(params[:target_code])
@@ -19,7 +23,7 @@ class SDGManagement::RelationsController < SDGManagement::BaseController
   def update
     @record.sdg_target_list = params[@record.class.table_name.singularize][:sdg_target_list]
 
-    redirect_to action: :index
+    redirect_to({ action: :index }, notice: update_notice)
   end
 
   private
@@ -39,4 +43,13 @@ class SDGManagement::RelationsController < SDGManagement::BaseController
       check_feature_flag(process_name)
       raise FeatureDisabled, process_name unless Setting["sdg.process.#{process_name}"]
     end
+
+    def update_notice
+      if @record.sdg_review.present?
+        t("sdg_management.relations.update.notice", relatable: relatable_class.model_name.human)
+      else
+        @record.create_sdg_review!
+        t("sdg_management.relations.update_and_review.notice", relatable: relatable_class.model_name.human)
+      end
+    end
 end

app/models/concerns/sdg/relatable.rb

@@ -10,6 +10,8 @@ module SDG::Relatable
                source: :related_sdg,
                source_type: sdg_type
     end
+
+    has_one :sdg_review, as: :relatable, dependent: :destroy, class_name: "SDG::Review"
   end
 
   class_methods do
@@ -26,6 +28,14 @@ module SDG::Relatable
 
       joins(sdg_class.table_name.to_sym).merge(sdg_class.where(code: code))
     end
+
+    def sdg_reviewed
+      joins(:sdg_review)
+    end
+
+    def pending_sdg_review
+      left_joins(:sdg_review).merge(SDG::Review.where(id: nil))
+    end
   end
 
   def related_sdgs

app/models/sdg/review.rb

@@ -0,0 +1,5 @@
+class SDG::Review < ApplicationRecord
+  validates :relatable_id, uniqueness: { scope: [:relatable_type] }
+
+  belongs_to :relatable, polymorphic: true, optional: false
+end

app/views/shared/_filter_subnav.html.erb

@@ -1,8 +1,8 @@
 <ul class="menu simple clear" id="filter-subnav">
   <li class="show-for-sr"><%= t("#{i18n_namespace}.filter") %>: </li>
 
-  <% @valid_filters.each do |filter| %>
-    <% if @current_filter == filter %>
+  <% valid_filters.each do |filter| %>
+    <% if current_filter == filter %>
       <li class="is-active"><h2><%= t("#{i18n_namespace}.filters.#{filter}") %></h2></li>
     <% else %>
       <li><%= link_to t("#{i18n_namespace}.filters.#{filter}"),

config/i18n-tasks.yml

@@ -219,6 +219,7 @@ ignore_unused:
   - "seeds.settings.*"
   - "dashboard.polls.*.submit"
   - "sdg.goals.goal_*"
+  - "sdg_management.relations.index.filter*"
 ####
 ## Exclude these keys from the `i18n-tasks eq-base" report:
 # ignore_eq_base:

config/locales/en/sdg_management.yml

@@ -32,3 +32,14 @@ en:
         title: "New local target"
       update:
         notice: "Local target updated successfully"
+    relations:
+      index:
+        filter: "Filter"
+        filters:
+          all: "All"
+          pending_sdg_review: "Pending"
+          sdg_reviewed: "Marked as reviewed"
+      update:
+        notice: "%{relatable} updated successfully"
+      update_and_review:
+        notice: "%{relatable} updated successfully and marked as reviewed"

config/locales/es/sdg_management.yml

@@ -32,3 +32,14 @@ es:
         title: "Nueva meta localizada"
       update:
         notice: "Meta localizada asctualizada correctamente"
+    relations:
+      index:
+        filter: "Filtro"
+        filters:
+          all: "Todos"
+          pending_sdg_review: "Pendientes"
+          sdg_reviewed: "Marcados como revisados"
+      update:
+        notice: "%{relatable} actualizado correctamente"
+      update_and_review:
+        notice: "%{relatable} actualizado correctamente y marcado como revisado"

db/migrate/20201221100246_create_sdg_reviews.rb

@@ -0,0 +1,8 @@
+class CreateSDGReviews < ActiveRecord::Migration[5.2]
+  def change
+    create_table :sdg_reviews do |t|
+      t.references :relatable, polymorphic: true, index: { unique: true }
+      t.timestamps
+    end
+  end
+end

db/schema.rb

@@ -1352,6 +1352,14 @@ ActiveRecord::Schema.define(version: 2021_01_07_125458) do
     t.index ["related_sdg_type", "related_sdg_id"], name: "index_sdg_relations_on_related_sdg_type_and_related_sdg_id"
   end
 
+  create_table "sdg_reviews", force: :cascade do |t|
+    t.string "relatable_type"
+    t.bigint "relatable_id"
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["relatable_type", "relatable_id"], name: "index_sdg_reviews_on_relatable_type_and_relatable_id", unique: true
+  end
+
   create_table "sdg_targets", force: :cascade do |t|
     t.bigint "goal_id"
     t.string "code", null: false

spec/components/sdg_management/relations/index_component_spec.rb

@@ -1,6 +1,16 @@
 require "rails_helper"
 
 describe SDGManagement::Relations::IndexComponent, type: :component do
+  before do
+    allow(ViewComponent::Base).to receive(:test_controller).and_return("SDGManagement::RelationsController")
+    allow_any_instance_of(SDGManagement::RelationsController).to receive(:valid_filters)
+      .and_return(SDGManagement::RelationsController::FILTERS)
+    allow_any_instance_of(SDGManagement::RelationsController).to receive(:current_filter)
+      .and_return(SDGManagement::RelationsController::FILTERS.first)
+    allow_any_instance_of(ApplicationHelper).to receive(:current_path_with_query_params)
+      .and_return("/anything")
+  end
+
   describe "#goal_options" do
     it "orders goals by code in the select" do
       component = SDGManagement::Relations::IndexComponent.new(Proposal.none.page(1))

spec/controllers/concerns/has_filters_spec.rb

@@ -6,7 +6,7 @@ describe HasFilters do
     has_filters ["all", "pending", "reviewed"], only: :index
 
     def index
-      render plain: "#{@current_filter} (#{@valid_filters.join(" ")})"
+      render plain: "#{current_filter} (#{valid_filters.join(" ")})"
     end
   end
 

spec/factories/sdg.rb

@@ -18,4 +18,13 @@ FactoryBot.define do
   factory :sdg_phase, class: "SDG::Phase" do
     kind { :sensitization }
   end
+
+  factory :sdg_review, class: "SDG::Review" do
+    SDG::Related::RELATABLE_TYPES.map { |relatable_type| relatable_type.downcase.gsub("::", "_") }
+    .each do |relatable|
+      trait :"#{relatable}_review" do
+        association :relatable, factory: relatable
+      end
+    end
+  end
 end

spec/models/sdg/relatable_spec.rb

@@ -117,6 +117,18 @@ describe SDG::Relatable do
     end
   end
 
+  describe "#sdg_review" do
+    it "returns nil when relatable is not reviewed" do
+      expect(relatable.sdg_review).to be_blank
+    end
+
+    it "returns the review when relatable is reviewed" do
+      review = create(:sdg_review, relatable: relatable)
+
+      expect(relatable.sdg_review).to eq(review)
+    end
+  end
+
   describe ".by_goal" do
     it "returns everything if no code is provided" do
       expect(relatable.class.by_goal("")).to eq [relatable]
@@ -158,4 +170,23 @@ describe SDG::Relatable do
       expect(relatable.class.by_target(target.code)).to be_empty
     end
   end
+
+  describe ".pending_sdg_review" do
+    let!(:relatable) { create(:proposal) }
+
+    it "returns records not reviewed yet" do
+      create(:sdg_review, relatable: create(:proposal))
+
+      expect(relatable.class.pending_sdg_review).to match_array [relatable]
+    end
+  end
+
+  describe ".sdg_reviewed" do
+    let!(:relatable) { create(:proposal) }
+    let!(:reviewed_relatable) { create(:sdg_review, relatable: create(:proposal)).relatable }
+
+    it "returns records already reviewed" do
+      expect(relatable.class.sdg_reviewed).to match_array [reviewed_relatable]
+    end
+  end
 end

spec/models/sdg/review_spec.rb

@@ -0,0 +1,23 @@
+require "rails_helper"
+
+describe SDG::Review do
+  describe "Validations" do
+    it "is valid for any given relatable" do
+      review = build(:sdg_review, :debate_review)
+
+      expect(review).to be_valid
+    end
+
+    it "is not valid without a relatable" do
+      review = build(:sdg_review, relatable: nil)
+
+      expect(review).not_to be_valid
+    end
+
+    it "is not valid when a review for given relatable already exists" do
+      relatable = create(:sdg_review, :proposal_review).relatable
+
+      expect(build(:sdg_review, relatable: relatable)).not_to be_valid
+    end
+  end
+end

spec/system/sdg_management/relations_spec.rb

@@ -18,26 +18,31 @@ describe "SDG Relations", :js do
 
     expect(page).to have_current_path "/sdg_management/budget/investments"
     expect(page).to have_css "h2", exact_text: "Participatory budgets"
+    expect(page).to have_css "li.is-active h2", exact_text: "Pending"
 
     within("#side_menu") { click_link "Debates" }
 
     expect(page).to have_current_path "/sdg_management/debates"
     expect(page).to have_css "h2", exact_text: "Debates"
+    expect(page).to have_css "li.is-active h2", exact_text: "Pending"
 
     within("#side_menu") { click_link "Collaborative legislation" }
 
     expect(page).to have_current_path "/sdg_management/legislation/processes"
     expect(page).to have_css "h2", exact_text: "Collaborative legislation"
+    expect(page).to have_css "li.is-active h2", exact_text: "Pending"
 
     within("#side_menu") { click_link "Polls" }
 
     expect(page).to have_current_path "/sdg_management/polls"
     expect(page).to have_css "h2", exact_text: "Polls"
+    expect(page).to have_css "li.is-active h2", exact_text: "Pending"
 
     within("#side_menu") { click_link "Proposals" }
 
     expect(page).to have_current_path "/sdg_management/proposals"
     expect(page).to have_css "h2", exact_text: "Proposals"
+    expect(page).to have_css "li.is-active h2", exact_text: "Pending"
   end
 
   describe "Index" do
@@ -88,6 +93,39 @@ describe "SDG Relations", :js do
       expect(page).to have_css "h2", exact_text: "Build a hospital"
     end
 
+    scenario "list records pending to review for the current model by default" do
+      create(:debate, title: "I'm a debate")
+      create(:sdg_review, relatable: create(:debate, title: "I'm a reviewed debate"))
+
+      visit sdg_management_debates_path
+
+      expect(page).to have_css "li.is-active h2", exact_text: "Pending"
+      expect(page).to have_text "I'm a debate"
+      expect(page).not_to have_text "I'm a reviewed debate"
+    end
+
+    scenario "list all records for the current model when user clicks on 'all' tab" do
+      create(:debate, title: "I'm a debate")
+      create(:sdg_review, relatable: create(:debate, title: "I'm a reviewed debate"))
+
+      visit sdg_management_debates_path
+      click_link "All"
+
+      expect(page).to have_text "I'm a debate"
+      expect(page).to have_text "I'm a reviewed debate"
+    end
+
+    scenario "list reviewed records for the current model when user clicks on 'reviewed' tab" do
+      create(:debate, title: "I'm a debate")
+      create(:sdg_review, relatable: create(:debate, title: "I'm a reviewed debate"))
+
+      visit sdg_management_debates_path
+      click_link "Marked as reviewed"
+
+      expect(page).not_to have_text "I'm a debate"
+      expect(page).to have_text "I'm a reviewed debate"
+    end
+
     describe "search" do
       scenario "search by terms" do
         create(:poll, name: "Internet speech freedom")
@@ -100,6 +138,7 @@ describe "SDG Relations", :js do
 
         expect(page).to have_content "Internet speech freedom"
         expect(page).not_to have_content "SDG interest"
+        expect(page).to have_css "li.is-active h2", exact_text: "Pending"
       end
 
       scenario "goal filter" do
@@ -112,34 +151,78 @@ describe "SDG Relations", :js do
 
         expect(page).to have_content "School"
         expect(page).not_to have_content "Hospital"
+        expect(page).to have_css "li.is-active h2", exact_text: "Pending"
       end
-    end
 
-    scenario "target filter" do
-      create(:budget_investment, title: "School", sdg_targets: [SDG::Target[4.1]])
-      create(:budget_investment, title: "Preschool", sdg_targets: [SDG::Target[4.2]])
+      scenario "target filter" do
+        create(:budget_investment, title: "School", sdg_targets: [SDG::Target[4.1]])
+        create(:budget_investment, title: "Preschool", sdg_targets: [SDG::Target[4.2]])
 
-      visit sdg_management_budget_investments_path
-      select "4.1", from: "target_code"
-      click_button "Search"
+        visit sdg_management_budget_investments_path
+        select "4.1", from: "target_code"
+        click_button "Search"
 
-      expect(page).to have_content "School"
-      expect(page).not_to have_content "Preschool"
+        expect(page).to have_content "School"
+        expect(page).not_to have_content "Preschool"
+        expect(page).to have_css "li.is-active h2", exact_text: "Pending"
+      end
+
+      scenario "search within current tab" do
+        visit sdg_management_proposals_path(filter: "pending_sdg_review")
+
+        click_button "Search"
+
+        expect(page).to have_css "li.is-active h2", exact_text: "Pending"
+
+        visit sdg_management_proposals_path(filter: "sdg_reviewed")
+
+        click_button "Search"
+
+        expect(page).to have_css "li.is-active h2", exact_text: "Marked as reviewed"
+
+        visit sdg_management_proposals_path(filter: "all")
+
+        click_button "Search"
+
+        expect(page).to have_css "li.is-active h2", exact_text: "All"
+      end
     end
   end
 
   describe "Edit" do
-    scenario "allows changing the targets" do
+    scenario "allows changing the targets and marks the resource as reviewed" do
       process = create(:legislation_process, title: "SDG process")
       process.sdg_targets = [SDG::Target["3.3"]]
 
       visit sdg_management_edit_legislation_process_path(process)
-      fill_in "Targets", with: "1.2, 2.1"
+      fill_in "Targets", with: "1.2, 2.1", fill_options: { clear: :backspace }
       click_button "Update Process"
 
+      expect(page).to have_content "Process updated successfully and marked as reviewed"
+
+      click_link "Marked as reviewed"
+
       within("tr", text: "SDG process") do
         expect(page).to have_css "td", exact_text: "1.2, 2.1"
       end
     end
+
+    scenario "does not show the review notice when resource was already reviewed" do
+      debate = create(:sdg_review, relatable: create(:debate, title: "SDG debate")).relatable
+      debate.sdg_targets = [SDG::Target["3.3"]]
+
+      visit sdg_management_edit_debate_path(debate, filter: "sdg_reviewed")
+      fill_in "Targets", with: "1.2, 2.1", fill_options: { clear: :backspace }
+      click_button "Update Debate"
+
+      expect(page).not_to have_content "Debate updated successfully and marked as reviewed"
+      expect(page).to have_content "Debate updated successfully"
+
+      click_link "Marked as reviewed"
+
+      within("tr", text: "SDG debate") do
+        expect(page).to have_css "td", exact_text: "1.2, 2.1"
+      end
+    end
   end
 end