From d9d23cb30a9da6498d05a3fb7d4fd128f6829ec8 Mon Sep 17 00:00:00 2001 From: Justin Littman Date: Thu, 16 Jan 2025 12:43:40 -0500 Subject: [PATCH] Adds review workflow. closes #209 --- Gemfile | 1 + Gemfile.lock | 8 ++ .../stylesheets/application.bootstrap.scss | 5 ++ .../pending_review_list_component.html.erb | 10 +++ .../show/pending_review_list_component.rb | 40 +++++++++ .../elements/alert_component.html.erb | 4 +- app/components/elements/alert_component.rb | 9 +- app/components/works/edit/submit_component.rb | 30 +++++++ .../works/show/review_component.html.erb | 16 ++++ app/components/works/show/review_component.rb | 20 +++++ .../show/review_rejected_component.html.erb | 10 +++ .../works/show/review_rejected_component.rb | 19 ++++ app/controllers/application_controller.rb | 6 +- app/controllers/dashboard_controller.rb | 2 +- app/controllers/works_controller.rb | 67 +++++++++++--- app/forms/review_form.rb | 10 +++ app/models/collection.rb | 4 + app/models/user.rb | 4 + app/models/work.rb | 21 +++++ app/policies/work_policy.rb | 15 +++- app/presenters/work_presenter.rb | 13 ++- app/views/dashboard/show.html.erb | 5 ++ app/views/works/form.html.erb | 3 +- app/views/works/show.html.erb | 4 + config/routes.rb | 6 +- ...20250116173206_add_review_state_to_work.rb | 7 ++ db/structure.sql | 12 ++- spec/components/show/header_component_spec.rb | 3 +- .../works/edit/submit_component_spec.rb | 62 +++++++++++++ spec/requests/review_work_spec.rb | 59 ++++++++++++ spec/system/create_work_review_spec.rb | 89 +++++++++++++++++++ spec/system/edit_work_spec.rb | 21 ++++- spec/system/review_accept_work_spec.rb | 68 ++++++++++++++ spec/system/review_reject_work_spec.rb | 48 ++++++++++ spec/system/show_dashboard_spec.rb | 13 ++- spec/system/show_work_spec.rb | 16 ++++ 36 files changed, 701 insertions(+), 29 deletions(-) create mode 100644 app/components/dashboard/show/pending_review_list_component.html.erb create mode 100644 app/components/dashboard/show/pending_review_list_component.rb create mode 100644 app/components/works/edit/submit_component.rb create mode 100644 app/components/works/show/review_component.html.erb create mode 100644 app/components/works/show/review_component.rb create mode 100644 app/components/works/show/review_rejected_component.html.erb create mode 100644 app/components/works/show/review_rejected_component.rb create mode 100644 app/forms/review_form.rb create mode 100644 db/migrate/20250116173206_add_review_state_to_work.rb create mode 100644 spec/components/works/edit/submit_component_spec.rb create mode 100644 spec/requests/review_work_spec.rb create mode 100644 spec/system/create_work_review_spec.rb create mode 100644 spec/system/review_accept_work_spec.rb create mode 100644 spec/system/review_reject_work_spec.rb diff --git a/Gemfile b/Gemfile index d61e4c7d..353eb328 100644 --- a/Gemfile +++ b/Gemfile @@ -53,6 +53,7 @@ gem 'frozen_record' # For licenses gem 'honeybadger' gem 'kaminari' # For pagination gem 'okcomputer' +gem 'state_machines-activerecord' gem 'view_component' gem 'whenever', require: false diff --git a/Gemfile.lock b/Gemfile.lock index fb344831..b027839d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -509,6 +509,13 @@ GEM net-sftp (>= 2.1.2) net-ssh (>= 2.8.0) ostruct + state_machines (0.6.0) + state_machines-activemodel (0.9.0) + activemodel (>= 6.0) + state_machines (>= 0.6.0) + state_machines-activerecord (0.9.0) + activerecord (>= 6.0) + state_machines-activemodel (>= 0.9.0) stimulus-rails (1.3.4) railties (>= 6.0.0) stringio (3.1.2) @@ -608,6 +615,7 @@ DEPENDENCIES solid_cable solid_cache solid_queue + state_machines-activerecord stimulus-rails turbo-rails tzinfo-data diff --git a/app/assets/stylesheets/application.bootstrap.scss b/app/assets/stylesheets/application.bootstrap.scss index 98ae9108..ad819ea7 100644 --- a/app/assets/stylesheets/application.bootstrap.scss +++ b/app/assets/stylesheets/application.bootstrap.scss @@ -161,4 +161,9 @@ table.table-treegrid { textarea { resize: both; +} + +// This is a custom alert variant that is used for the review alert. +.alert.alert-input { + background-color: var(--stanford-10-black); } \ No newline at end of file diff --git a/app/components/dashboard/show/pending_review_list_component.html.erb b/app/components/dashboard/show/pending_review_list_component.html.erb new file mode 100644 index 00000000..e37eeb0c --- /dev/null +++ b/app/components/dashboard/show/pending_review_list_component.html.erb @@ -0,0 +1,10 @@ +<%= render Elements::Tables::TableComponent.new( + id:, label: 'Drafts', show_label: false + ) do |component| %> + <% component.with_header( + headers: ['Deposit', 'Collection', 'Owner', 'Last modified'] + ) %> + <% works.each do |work| %> + <% component.with_row(values: values_for(work), id: id_for(work)) %> + <% end %> +<% end %> diff --git a/app/components/dashboard/show/pending_review_list_component.rb b/app/components/dashboard/show/pending_review_list_component.rb new file mode 100644 index 00000000..15ea923b --- /dev/null +++ b/app/components/dashboard/show/pending_review_list_component.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Dashboard + module Show + # Component for rendering a table on the work show page with works pending review. + class PendingReviewListComponent < ApplicationComponent + def initialize(works:) + @works = works + super() + end + + attr_reader :works + + def id + 'pending-review-table' + end + + def id_for(work) + dom_id(work, id) + end + + def values_for(work) + [ + link_to(work.title, link_for(work)), + link_to(work.collection.title, collection_path(druid: work.collection.druid)), + work.user.name, + I18n.l(work.updated_at, format: '%b %d, %Y') + ] + end + + private + + def link_for(work) + return wait_works_path(work) unless work.druid + + work_path(druid: work.druid) + end + end + end +end diff --git a/app/components/elements/alert_component.html.erb b/app/components/elements/alert_component.html.erb index 033b1f30..047df8e3 100644 --- a/app/components/elements/alert_component.html.erb +++ b/app/components/elements/alert_component.html.erb @@ -1,5 +1,7 @@ <%= tag.div role: 'alert', class: classes, data: do %> -
<%= icon %>
+ <% if icon? %> +
<%= icon %>
+ <% end %>
<% if title.present? %>
<%= title %>
diff --git a/app/components/elements/alert_component.rb b/app/components/elements/alert_component.rb index 1949e44a..fb5fcff9 100644 --- a/app/components/elements/alert_component.rb +++ b/app/components/elements/alert_component.rb @@ -3,9 +3,10 @@ module Elements # Component for rendering an alert. class AlertComponent < ApplicationComponent - # Variants are :danger, :success, :note, :info, :warning + # Variants are :danger, :success, :note, :info, :warning, :input + # input is not part of the component library def initialize(title: nil, variant: :info, dismissible: false, value: nil, data: {}, classes: []) # rubocop:disable Metrics/ParameterLists - raise ArgumentError, 'Invalid variant' unless %i[danger success note info warning].include?(variant.to_sym) + raise ArgumentError, 'Invalid variant' unless %i[danger success note info warning input].include?(variant.to_sym) @title = title @variant = variant @@ -34,6 +35,10 @@ def dismissible? @dismissible end + def icon? + helpers.respond_to?(:"#{variant}_icon") + end + def icon helpers.public_send(:"#{variant}_icon") end diff --git a/app/components/works/edit/submit_component.rb b/app/components/works/edit/submit_component.rb new file mode 100644 index 00000000..e2cdfad3 --- /dev/null +++ b/app/components/works/edit/submit_component.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module Works + module Edit + # Component for rendering a submit button + class SubmitComponent < ApplicationComponent + def initialize(form_id:, work:, collection:, classes: []) + @form_id = form_id + @work = work + @collection = collection + @classes = classes + super() + end + + attr_reader :form_id, :work, :collection, :classes + + def call + render Elements::Forms::SubmitComponent.new(form_id:, label:, classes:) + end + + def label + if collection.review_enabled? && (work.nil? || !helpers.allowed_to?(:review?, work)) + 'Submit for review' + else + 'Deposit' + end + end + end + end +end diff --git a/app/components/works/show/review_component.html.erb b/app/components/works/show/review_component.html.erb new file mode 100644 index 00000000..a2cbfbbb --- /dev/null +++ b/app/components/works/show/review_component.html.erb @@ -0,0 +1,16 @@ +<%= render Elements::AlertComponent.new(variant: :input) do %> +
Review all details below then approve this deposit or return with comments.
+ <%= form_with model: review_form, url: review_work_path, method: :put do |form| %> +
+ <%= form.radio_button :review_option, 'approve', class: 'form-check-input' %> + <%= form.label :review_option_approve, 'Approve', class: 'form-check-label' %> +
+
+ <%= form.radio_button :review_option, 'reject', class: 'form-check-input' %> + <%= form.label :review_option_reject, 'Return with comments', class: 'form-check-label' %> + <%= render Elements::Forms::TextareaFieldComponent.new(form:, field_name: :reject_reason, label: 'Reason for returning', hidden_label: true) %> +
+ + <%= render Elements::Forms::SubmitComponent.new(label: 'Submit') %> + <% end %> +<% end %> diff --git a/app/components/works/show/review_component.rb b/app/components/works/show/review_component.rb new file mode 100644 index 00000000..37e5c747 --- /dev/null +++ b/app/components/works/show/review_component.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module Works + module Show + # Component providing a form for submitting a review. + class ReviewComponent < ApplicationComponent + def initialize(work:, review_form:) + @work = work + @review_form = review_form + super() + end + + attr_reader :review_form + + def render? + @work.pending_review? && helpers.allowed_to?(:review?, @work) + end + end + end +end diff --git a/app/components/works/show/review_rejected_component.html.erb b/app/components/works/show/review_rejected_component.html.erb new file mode 100644 index 00000000..18f0fcba --- /dev/null +++ b/app/components/works/show/review_rejected_component.html.erb @@ -0,0 +1,10 @@ +<%= render Elements::AlertComponent.new(variant: :danger) do %> +

+ The reviewer for this collection has returned the deposit for the following reason(s). + + Please fix the items given below and submit your deposit again for review. +

+
+

<%= review_rejected_reason %>

+
+<% end %> diff --git a/app/components/works/show/review_rejected_component.rb b/app/components/works/show/review_rejected_component.rb new file mode 100644 index 00000000..6e5dccbf --- /dev/null +++ b/app/components/works/show/review_rejected_component.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Works + module Show + # Component for rendering an alert for a rejected review. + class ReviewRejectedComponent < ApplicationComponent + def initialize(work:) + @work = work + super() + end + + delegate :review_rejected_reason, to: :@work + + def render? + @work.rejected_review? + end + end + end +end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 1c7e6823..d96d106f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -26,9 +26,13 @@ def deposit? params[:commit] == 'Deposit' end + def request_review? + params[:commit] == 'Submit for review' + end + # NOTE: a `nil` validation context runs all validations without an explicit context def validation_context - return :deposit if deposit? + return :deposit if deposit? || request_review? nil end diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index dd24579c..1b44462f 100644 --- a/app/controllers/dashboard_controller.rb +++ b/app/controllers/dashboard_controller.rb @@ -19,6 +19,6 @@ def show private def druids_for_works_for_current_user - current_user.works.where.not(druid: nil).pluck(:druid) + current_user.your_works.where.not(druid: nil).pluck(:druid) end end diff --git a/app/controllers/works_controller.rb b/app/controllers/works_controller.rb index 8484ee87..98eee359 100644 --- a/app/controllers/works_controller.rb +++ b/app/controllers/works_controller.rb @@ -1,13 +1,13 @@ # frozen_string_literal: true # Controller for a Work -class WorksController < ApplicationController - before_action :set_work, only: %i[show edit update destroy] +class WorksController < ApplicationController # rubocop:disable Metrics/ClassLength + before_action :set_work, only: %i[show edit update destroy review] before_action :check_deposit_job_started, only: %i[show edit] - before_action :set_work_form_from_cocina, only: %i[show edit] - before_action :set_content, only: %i[show edit] - before_action :set_status, only: %i[show edit destroy] - before_action :set_presenter, only: %i[show edit] + before_action :set_work_form_from_cocina, only: %i[show edit review] + before_action :set_content, only: %i[show edit review] + before_action :set_status, only: %i[show edit destroy review] + before_action :set_presenter, only: %i[show edit review] def show authorize! @work @@ -15,6 +15,8 @@ def show # This updates the Work with the latest metadata from the Cocina object. # Does not update the Work's collection if the collection cannot be found. ModelSync::Work.call(work: @work, cocina_object: @cocina_object, raise: false) + + @review_form = ReviewForm.new end def new @@ -52,11 +54,9 @@ def create # rubocop:disable Metrics/AbcSize # The validation_context param determines whether extra validations are applied, e.g., for deposits. if @work_form.valid?(validation_context) - # Setting the deposit_job_started_at to the current time to indicate that the deposit job has started and user - # should be "waiting". work = Work.create!(title: @work_form.title, user: current_user, deposit_job_started_at: Time.zone.now, collection: @collection) - DepositWorkJob.perform_later(work:, work_form: @work_form, deposit: deposit?) + perform_deposit(work:) redirect_to wait_works_path(work.id) else @content = Content.find(@work_form.content_id) @@ -65,16 +65,13 @@ def create # rubocop:disable Metrics/AbcSize end end - def update # rubocop:disable Metrics/AbcSize + def update authorize! @work @work_form = WorkForm.new(**update_work_params) # The validation_context param determines whether extra validations are applied, e.g., for deposits. if @work_form.valid?(validation_context) - # Setting the deposit_job_started_at to the current time to indicate that the deposit job has started and user - # should be "waiting". - @work.update!(deposit_job_started_at: Time.zone.now) - DepositWorkJob.perform_later(work: @work, work_form: @work_form, deposit: deposit?) + perform_deposit(work: @work) redirect_to wait_works_path(@work.id) else @content = Content.find(@work_form.content_id) @@ -106,12 +103,28 @@ def wait redirect_to work_path(druid: work.druid) if work.deposit_job_finished? end + def review + authorize! @work + + @review_form = ReviewForm.new(**review_form_params) + if @review_form.valid? + redirect_path = perform_review + redirect_to redirect_path + else + render :show, status: :unprocessable_entity + end + end + private def work_params params.expect(work: WorkForm.user_editable_attributes + [WorkForm.nested_attributes]) end + def review_form_params + params.expect(review: %i[review_option reject_reason]) + end + def update_work_params work_params.merge(druid: params[:druid]) end @@ -168,4 +181,30 @@ def new_work_form release_date: @collection.max_release_date ) end + + def perform_deposit(work:) + # Setting the deposit_job_started_at to the current time to indicate that the deposit job has started and user + # should be "waiting". + work.update!(deposit_job_started_at: Time.zone.now) + deposit = deposit? + if request_review? + work.request_review! + deposit = false # Will be saved, but not deposited until approved. + end + DepositWorkJob.perform_later(work:, work_form: @work_form, deposit:) + end + + # @return [String] path to redirect to after review + def perform_review + if @review_form.review_option == 'approve' + # Deposit + @work.update!(deposit_job_started_at: Time.zone.now) + @work.approve! + DepositWorkJob.perform_later(work: @work, work_form: @work_form, deposit: true) + wait_works_path(@work.id) + else + @work.reject_with_reason!(reason: @review_form.reject_reason) + work_path(druid: @work.druid) + end + end end diff --git a/app/forms/review_form.rb b/app/forms/review_form.rb new file mode 100644 index 00000000..8efb3ed4 --- /dev/null +++ b/app/forms/review_form.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +# Form object for reviewing a work. +class ReviewForm < ApplicationForm + attribute :review_option, :string, default: 'approve' + validates :review_option, inclusion: { in: %w[approve reject] } + + attribute :reject_reason, :string + validates :reject_reason, presence: true, if: -> { review_option == 'reject' } +end diff --git a/app/models/collection.rb b/app/models/collection.rb index 767352d9..6c527b03 100644 --- a/app/models/collection.rb +++ b/app/models/collection.rb @@ -54,4 +54,8 @@ def max_release_date def add_user_as_manager managers << user if user.present? && managers.exclude?(user) end + + def review_enabled? + review_enabled + end end diff --git a/app/models/user.rb b/app/models/user.rb index c9f00433..c3185013 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -28,4 +28,8 @@ def your_collections def your_works Work.where(collection: your_collections) end + + def your_pending_review_works + Work.where(collection: reviewer_for).with_review_state(:pending_review) + end end diff --git a/app/models/work.rb b/app/models/work.rb index 351a8a55..a0be4440 100644 --- a/app/models/work.rb +++ b/app/models/work.rb @@ -6,6 +6,27 @@ class Work < ApplicationRecord belongs_to :user belongs_to :collection + state_machine :review_state, initial: :none do + event :request_review do + transition none: :pending_review + transition rejected_review: :pending_review + end + + event :approve do + transition pending_review: :none + end + + event :reject do + transition pending_review: :rejected_review + end + end + + def reject_with_reason!(reason:) + self.review_rejected_reason = reason + self.review_state_event = 'reject' + save! + end + # deposit_job_started_at indicates that the job is queued or running. # User should be "waiting" until the job is completed. def deposit_job_started? diff --git a/app/policies/work_policy.rb b/app/policies/work_policy.rb index 13997ed8..a109a286 100644 --- a/app/policies/work_policy.rb +++ b/app/policies/work_policy.rb @@ -1,12 +1,21 @@ # frozen_string_literal: true class WorkPolicy < ApplicationPolicy - alias_rule :new?, :create?, :show?, :update?, :edit?, :wait?, :destroy?, to: :manage? + alias_rule :new?, :create?, :update?, :edit?, :destroy?, to: :manage? + alias_rule :wait?, to: :show? def manage? record.user_id == user.id || collection_manager? || collection_owner? || collection_depositor? end + def show? + manage? || collection_reviewer? + end + + def review? + collection_reviewer? || collection_manager? + end + def collection_manager? return record.managers.include?(user) if record.is_a? Collection @@ -22,4 +31,8 @@ def collection_depositor? def collection_owner? record.collection.user_id == user.id if record.is_a? Work end + + def collection_reviewer? + record.collection.reviewers.include?(user) + end end diff --git a/app/presenters/work_presenter.rb b/app/presenters/work_presenter.rb index b902246c..edbd2b03 100644 --- a/app/presenters/work_presenter.rb +++ b/app/presenters/work_presenter.rb @@ -73,9 +73,20 @@ def doi_value end end + def status_message + case review_state + when 'pending_review' + 'Pending review' + when 'rejected_review' + 'Returned' + else + super + end + end + private - delegate :collection, :created_at, :user, to: :work + delegate :collection, :created_at, :user, :review_state, to: :work def license_presenter @license_presenter ||= LicensePresenter.new(work_form:, collection:) diff --git a/app/views/dashboard/show.html.erb b/app/views/dashboard/show.html.erb index 09224c1f..c247898f 100644 --- a/app/views/dashboard/show.html.erb +++ b/app/views/dashboard/show.html.erb @@ -7,6 +7,11 @@
<% end %> +<% if current_user.your_pending_review_works.exists? %> + <%= render Elements::HeadingComponent.new(level: :h3, text: 'Deposits that are pending review', classes: 'mb-4 w-100 text-nowrap') %> + <%= render Dashboard::Show::PendingReviewListComponent.new(works: current_user.your_pending_review_works) %> +<% end %> +
<%= render Elements::HeadingComponent.new(level: :h3, text: 'Your collections', classes: 'mb-4 w-100 text-nowrap') %> <%= render Elements::ButtonLinkComponent.new(link: new_collection_path, label: 'Create a new collection', variant: :primary, classes: 'ml-auto text-nowrap h-50') if allowed_to?(:new?, Collection) %> diff --git a/app/views/works/form.html.erb b/app/views/works/form.html.erb index b4963b9f..144f31dd 100644 --- a/app/views/works/form.html.erb +++ b/app/views/works/form.html.erb @@ -10,6 +10,7 @@ <% end %> <% end %> <%= render Elements::HeadingComponent.new(level: :h1, variant: :h2, text: @work_form.title || t('works.edit.no_title'), classes: 'mb-3') %> +<%= render Works::Show::ReviewRejectedComponent.new(work: @work) if @work %> <%= render Elements::AlertComponent.new(variant: :danger, value: t('messages.validation')) if @work_form.errors.present? %> <% form_id = dom_id(@work_form, 'tabbed_form') %> <%= render Elements::TabForm::TabListComponent.new(model: @work_form, form_id:, hidden_fields: %i[lock version collection_druid content_id]) do |component| %> @@ -96,7 +97,7 @@
<%= render Elements::CancelButtonComponent.new(link: root_path, classes: 'me-4') %> - <%= render Elements::Forms::SubmitComponent.new(form_id:, label: 'Deposit', classes: 'ms-2') %> + <%= render Works::Edit::SubmitComponent.new(form_id:, work: @work, collection: @collection, classes: 'ms-2') %>
<%= render Edit::DiscardDraftButtonComponent.new(presenter: @work_presenter) if @work_presenter %>
diff --git a/app/views/works/show.html.erb b/app/views/works/show.html.erb index dcb5a47b..e82ee579 100644 --- a/app/views/works/show.html.erb +++ b/app/views/works/show.html.erb @@ -4,6 +4,10 @@ <% component.with_breadcrumb(text: @work_presenter.title) %> <% end %> <% end %> + +<%= render Works::Show::ReviewComponent.new(work: @work, review_form: @review_form) %> +<%= render Works::Show::ReviewRejectedComponent.new(work: @work) %> + <%= render Show::HeaderComponent.new(presenter: @work_presenter) %> <%= render Elements::Tables::TableComponent.new(id: 'details-table', classes: 'mb-5', label: 'Details') do |component| %> diff --git a/config/routes.rb b/config/routes.rb index 53d14585..ec44140b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -Rails.application.routes.draw do +Rails.application.routes.draw do # rubocop:disable Metrics/BlockLength # Define your application routes per the DSL in https://guides.rubyonrails.org/routing.html get '/webauth/login', to: 'authentication#login', as: 'login' get '/webauth/logout', to: 'authentication#logout', as: 'logout' @@ -26,6 +26,10 @@ collection do get 'wait/:id', to: 'works#wait', as: 'wait' end + + member do + put 'review', to: 'works#review', as: 'review' + end end resources :contents, only: %i[update show] do diff --git a/db/migrate/20250116173206_add_review_state_to_work.rb b/db/migrate/20250116173206_add_review_state_to_work.rb new file mode 100644 index 00000000..9cd2eb60 --- /dev/null +++ b/db/migrate/20250116173206_add_review_state_to_work.rb @@ -0,0 +1,7 @@ +class AddReviewStateToWork < ActiveRecord::Migration[8.0] + def change + add_column :works, :review_state, :string, default: 'none', null: false + add_column :works, :review_rejected_reason, :string + add_index :works, :review_state + end +end diff --git a/db/structure.sql b/db/structure.sql index b17d44d3..62bb3286 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -332,7 +332,9 @@ CREATE TABLE public.works ( updated_at timestamp(6) without time zone NOT NULL, collection_id bigint NOT NULL, object_updated_at timestamp(6) without time zone, - doi_assigned boolean DEFAULT false NOT NULL + doi_assigned boolean DEFAULT false NOT NULL, + review_state character varying DEFAULT 'none'::character varying NOT NULL, + review_rejected_reason character varying ); @@ -610,6 +612,13 @@ CREATE UNIQUE INDEX index_works_on_druid ON public.works USING btree (druid); CREATE INDEX index_works_on_object_updated_at ON public.works USING btree (object_updated_at); +-- +-- Name: index_works_on_review_state; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX index_works_on_review_state ON public.works USING btree (review_state); + + -- -- Name: index_works_on_user_id; Type: INDEX; Schema: public; Owner: - -- @@ -680,6 +689,7 @@ ALTER TABLE ONLY public.active_storage_attachments SET search_path TO "$user", public; INSERT INTO "schema_migrations" (version) VALUES +('20250116173206'), ('20250114195340'), ('20250108112958'), ('20250107153556'), diff --git a/spec/components/show/header_component_spec.rb b/spec/components/show/header_component_spec.rb index 6b2159a9..fbe2f292 100644 --- a/spec/components/show/header_component_spec.rb +++ b/spec/components/show/header_component_spec.rb @@ -4,9 +4,10 @@ RSpec.describe Show::HeaderComponent, type: :component do let(:presenter) do - WorkPresenter.new(work_form:, version_status:, work: nil) + WorkPresenter.new(work_form:, version_status:, work:) end let(:work_form) { WorkForm.new(druid: druid_fixture, title:) } + let(:work) { instance_double(Work, review_state: 'none') } let(:version_status) do instance_double(VersionStatus, editable?: editable, discardable?: discardable, status_message:) end diff --git a/spec/components/works/edit/submit_component_spec.rb b/spec/components/works/edit/submit_component_spec.rb new file mode 100644 index 00000000..a39ecb5d --- /dev/null +++ b/spec/components/works/edit/submit_component_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Works::Edit::SubmitComponent, type: :component do + let(:form_id) { 'new_work' } + let(:user) { create(:user) } + let(:reviewer) { create(:user) } + let(:collection) { create(:collection, review_enabled:, reviewers: [reviewer]) } + let(:work) { create(:work, user:, collection:) } + let(:review_enabled) { true } + + before do + allow(Current).to receive(:groups).and_return([]) + end + + context 'when work is nil (for a create)' do + let(:required) { false } + + it 'renders the submit' do + render_inline(described_class.new(form_id:, work: nil, collection:)) + + expect(page).to have_button('Submit for review') + end + end + + context 'when review is not enabled' do + let(:review_enabled) { false } + + it 'renders the submit' do + render_inline(described_class.new(form_id:, work:, collection:)) + + expect(page).to have_button('Deposit') + end + end + + context 'when user is work owner' do + before do + allow(vc_test_controller).to receive(:current_user).and_return(user) + allow(Current).to receive(:groups).and_return([]) + end + + it 'renders the submit' do + render_inline(described_class.new(form_id:, work:, collection:)) + + expect(page).to have_button('Submit for review') + end + end + + context 'when user is reviewer' do + before do + allow(vc_test_controller).to receive(:current_user).and_return(reviewer) + allow(Current).to receive(:groups).and_return([]) + end + + it 'renders the submit' do + render_inline(described_class.new(form_id:, work:, collection:)) + + expect(page).to have_button('Deposit') + end + end +end diff --git a/spec/requests/review_work_spec.rb b/spec/requests/review_work_spec.rb new file mode 100644 index 00000000..f4effaff --- /dev/null +++ b/spec/requests/review_work_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Review work' do + include WorkMappingFixtures + + let(:druid) { druid_fixture } + + let(:manager) { create(:user) } + let(:reviewer) { create(:user) } + + before do + allow(Sdr::Repository).to receive(:find).with(druid:).and_return(dro_with_metadata_fixture) + allow(Sdr::Repository).to receive(:status) + .with(druid:).and_return(instance_double(Dor::Services::Client::ObjectVersion::VersionStatus)) + + collection = create(:collection, druid: collection_druid_fixture, managers: [manager], reviewers: [reviewer]) + create(:work, druid:, collection:) + end + + context 'when the user is not authorized' do + before do + sign_in(create(:user)) + end + + it 'redirects to root' do + put "/works/#{druid}/review" + + expect(response).to redirect_to(root_path) + end + end + + context 'when the user is a manager' do + before do + sign_in(manager) + end + + it 'returns 400' do + put "/works/#{druid}/review" + + # Bad request indicates that the user is authorized. + expect(response).to have_http_status(:bad_request) + end + end + + context 'when the user is a reviewer' do + before do + sign_in(reviewer) + end + + it 'returns 400' do + put "/works/#{druid}/review" + + # Bad request indicates that the user is authorized. + expect(response).to have_http_status(:bad_request) + end + end +end diff --git a/spec/system/create_work_review_spec.rb b/spec/system/create_work_review_spec.rb new file mode 100644 index 00000000..4009c0fc --- /dev/null +++ b/spec/system/create_work_review_spec.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Create a work that requires review' do + include_context 'with FAST connection' + + let(:query) { 'Biology' } + let(:druid) { druid_fixture } + let(:user) { create(:user) } + let(:version_status) do + VersionStatus.new(status: + instance_double(Dor::Services::Client::ObjectVersion::VersionStatus, open?: false, accessioning?: true, + openable?: false, version: 1)) + end + + before do + # Stubbing out for Deposit Job + allow(Sdr::Repository).to receive(:register) do |args| + cocina_params = args[:cocina_object].to_h + cocina_params[:externalIdentifier] = druid + cocina_params[:description][:purl] = Sdr::Purl.from_druid(druid:) + cocina_params[:structural] = { isMemberOf: [collection_druid_fixture] } + cocina_params[:administrative] = { hasAdminPolicy: Settings.apo } + cocina_object = Cocina::Models.build(cocina_params) + (@registered_cocina_object = Cocina::Models.with_metadata(cocina_object, 'abc123')) + end + allow(Sdr::Repository).to receive(:accession) + + # Stubbing out for show page + allow(Sdr::Repository).to receive(:find).with(druid:).and_invoke(->(_arg) { @registered_cocina_object }) + allow(Sdr::Repository).to receive(:status).with(druid:).and_return(version_status) + create(:collection, :with_review_workflow, user:, druid: collection_druid_fixture, managers: [user]) + sign_in(user) + end + + it 'creates and deposits a work' do + visit root_path + click_link_or_button('Deposit to this collection') + + expect(page).to have_css('h1', text: 'Untitled deposit') + + # Adding a file + find('.dropzone').drop('spec/fixtures/files/hippo.png') + + expect(page).to have_css('table#content-table td', text: 'hippo.png') + + # Filling in title + find('.nav-link', text: 'Title & contact').click + fill_in('work_title', with: title_fixture) + fill_in('Contact email', with: contact_emails_fixture.first['email']) + + # Click Next to go to authors tab + click_link_or_button('Next') + expect(page).to have_css('.nav-link.active', text: 'Authors') + + # Enter an authors + select('Creator', from: 'work_authors_attributes_0_person_role') + fill_in('First name', with: 'Jane') + fill_in('Last name', with: 'Stanford') + + # Click Next to go to abstract tab + click_link_or_button('Next') + expect(page).to have_css('.nav-link.active', text: 'Abstract') + + # Filling in abstract & keywords + fill_in('work_abstract', with: abstract_fixture) + fill_in('Keywords (one per box)', with: keywords_fixture.first['text']) + + # Click Next to go to work type tab + click_link_or_button('Next') + expect(page).to have_css('.nav-link.active', text: 'Type of deposit') + + # Selecting work type + choose('Text') + check('Thesis') + + # Clicking on Next to go to Deposit + find('.nav-link', text: 'Deposit').click + expect(page).to have_css('.nav-link.active', text: 'Deposit') + click_link_or_button('Submit for review') + + # Waiting page may be too fast to catch so not testing. + # On show page + expect(page).to have_css('h1', text: title_fixture) + expect(page).to have_css('.status', text: 'Pending review') + expect(page).to have_no_link('Edit or deposit') + end +end diff --git a/spec/system/edit_work_spec.rb b/spec/system/edit_work_spec.rb index faac0f42..399ce8bc 100644 --- a/spec/system/edit_work_spec.rb +++ b/spec/system/edit_work_spec.rb @@ -51,6 +51,9 @@ ] end + let(:collection) { create(:collection, user:, druid: collection_druid_fixture, title: collection_title_fixture) } + let!(:work) { create(:work, druid:, user:, collection:) } + before do # On the second call, this will return the cocina object submitted to update. # This will allow us to test the updated values. @@ -63,8 +66,6 @@ allow(Sdr::Repository).to receive(:update) do |args| @updated_cocina_object = args[:cocina_object] end - collection = create(:collection, user:, druid: collection_druid_fixture, title: collection_title_fixture) - create(:work, druid:, user:, collection:) sign_in(user) end @@ -151,4 +152,20 @@ expect(page).to have_css('.status', text: 'New version in draft') expect(page).to have_link('Edit or deposit', href: edit_work_path(druid)) end + + context 'when rejected' do + before do + work.request_review! + work.reject_with_reason!(reason: 'Try harder.') + end + + it 'shows a work with rejected alert' do + visit edit_work_path(druid) + + within('.alert') do + expect(page).to have_text('The reviewer for this collection has returned the deposit') + expect(page).to have_css('blockquote', text: 'Try harder.') + end + end + end end diff --git a/spec/system/review_accept_work_spec.rb b/spec/system/review_accept_work_spec.rb new file mode 100644 index 00000000..ce57adfe --- /dev/null +++ b/spec/system/review_accept_work_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Review and accept a work' do + include WorkMappingFixtures + + let(:druid) { druid_fixture } + let(:user) { create(:user) } + let(:collection) { create(:collection, druid: collection_druid_fixture, reviewers: [user], review_enabled: true) } + let!(:work) { create(:work, druid:, title: title_fixture, collection:, review_state: 'pending_review') } + let(:cocina_object) { dro_with_metadata_fixture } + let(:version_status) do + VersionStatus.new( + status: instance_double(Dor::Services::Client::ObjectVersion::VersionStatus, open?: true, version: 2, + openable?: false, + accessioning?: false, + discardable?: false) + ) + end + + let(:deposit_version_status) do + VersionStatus.new( + status: instance_double(Dor::Services::Client::ObjectVersion::VersionStatus, open?: false, version: 2, + openable?: false, + accessioning?: true, + discardable?: false) + ) + end + + before do + # Stubbing out for Deposit Job + allow(Sdr::Repository).to receive(:open_if_needed) { |args| args[:cocina_object] } + allow(Sdr::Repository).to receive(:update) { |args| args[:cocina_object] } + allow(Sdr::Repository).to receive(:accession) + + allow(Sdr::Repository).to receive(:find).with(druid:).and_return(cocina_object) + allow(Sdr::Repository).to receive(:status).with(druid:).and_return(version_status, deposit_version_status) + + sign_in(user) + end + + it 'reviews a work' do + visit work_path(druid) + + expect(page).to have_css('h1', text: work.title) + expect(page).to have_css('.status', text: 'Pending review') + expect(page).to have_text('Review all details below then approve this deposit or return with comments.') + + expect(page).to have_checked_field('Approve') + + # Submit invalid (reason is required) + choose('Return with comments') + click_on('Submit') + + expect(page).to have_css('.invalid-feedback', text: "can't be blank") + + # Approve and submit + choose('Approve') + click_on('Submit') + + # Waiting page may be too fast to catch so not testing. + # On show page + expect(page).to have_css('h1', text: title_fixture) + expect(page).to have_css('.status', text: 'Depositing') + expect(page).to have_no_text('Review all details below then approve or reject this deposit.') + end +end diff --git a/spec/system/review_reject_work_spec.rb b/spec/system/review_reject_work_spec.rb new file mode 100644 index 00000000..a80d7623 --- /dev/null +++ b/spec/system/review_reject_work_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Review and reject a work' do + include WorkMappingFixtures + + let(:druid) { druid_fixture } + let(:user) { create(:user) } + let(:collection) { create(:collection, druid: collection_druid_fixture, reviewers: [user], review_enabled: true) } + let!(:work) { create(:work, druid:, title: title_fixture, collection:, review_state: 'pending_review') } + let(:cocina_object) { dro_with_metadata_fixture } + let(:version_status) do + VersionStatus.new( + status: instance_double(Dor::Services::Client::ObjectVersion::VersionStatus, open?: true, version: 2, + openable?: false, + accessioning?: false, + discardable?: false) + ) + end + + before do + allow(Sdr::Repository).to receive(:find).with(druid:).and_return(cocina_object) + allow(Sdr::Repository).to receive(:status).with(druid:).and_return(version_status) + + sign_in(user) + end + + it 'reviews a work' do + visit work_path(druid) + + expect(page).to have_css('h1', text: work.title) + expect(page).to have_css('.status', text: 'Pending review') + expect(page).to have_text('Review all details below then approve this deposit or return with comments.') + + expect(page).to have_checked_field('Approve') + + # Submit invalid (reason is required) + choose('Return with comments') + fill_in('Reason for returning', with: "I don't like the cut of your jib.") + click_on('Submit') + + # On show page + expect(page).to have_css('h1', text: title_fixture) + expect(page).to have_css('.status', text: 'Returned') + expect(page).to have_no_text('Review all details below then approve or reject this deposit.') + end +end diff --git a/spec/system/show_dashboard_spec.rb b/spec/system/show_dashboard_spec.rb index 0d27be09..64b851d3 100644 --- a/spec/system/show_dashboard_spec.rb +++ b/spec/system/show_dashboard_spec.rb @@ -6,7 +6,10 @@ let!(:work) { create(:work, :with_druid, user:, collection:) } let!(:work_without_druid) { create(:work, user:, collection:) } let!(:draft_work) { create(:work, :with_druid, user:, collection:) } - let(:collection) { create(:collection, :with_druid, user:, managers: [user]) } + let!(:pending_review_work) { create(:work, user:, collection:, review_state: 'pending_review') } + let(:collection) do + create(:collection, :with_druid, user:, managers: [user], reviewers: [user], review_enabled: true) + end let(:user) { create(:user) } let(:version_status) do VersionStatus.new(status: @@ -24,7 +27,7 @@ sign_in(user) end - it 'displays the user name in the header' do + it 'displays the dashboard' do visit root_path expect(page).to have_css('h2', text: "#{user.name} - Dashboard") @@ -35,6 +38,12 @@ expect(page).to have_css('td', text: draft_work.title) end + # Pending review section + expect(page).to have_css('h3', text: 'Deposits that are pending review') + within('table#pending-review-table') do + expect(page).to have_css('td', text: pending_review_work.title) + end + # Your collections section expect(page).to have_css('h3', text: 'Your collections') expect(page).to have_css('h4', text: collection.title) diff --git a/spec/system/show_work_spec.rb b/spec/system/show_work_spec.rb index 8dc46e75..1b23d52f 100644 --- a/spec/system/show_work_spec.rb +++ b/spec/system/show_work_spec.rb @@ -288,4 +288,20 @@ end end end + + context 'when rejected' do + before do + work.request_review! + work.reject_with_reason!(reason: 'Try harder.') + end + + it 'shows a work with rejected alert' do + visit work_path(druid) + + within('.alert') do + expect(page).to have_text('The reviewer for this collection has returned the deposit') + expect(page).to have_css('blockquote', text: 'Try harder.') + end + end + end end