Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD] Edit project tag from the show page #3150

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/components/show/agreement/details_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<tr>
<th class="col-3" scope="row">Project</th>
<td>
<%= render Show::ProjectTagComponent.new(document: @presenter.document) %>
<%= render Show::ProjectTagComponent.new(change_set: @presenter.change_set) %>
</td>
</tr>

Expand Down
2 changes: 1 addition & 1 deletion app/components/show/collection/details_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<tr>
<th class="col-3" scope="row">Project</th>
<td>
<%= render Show::ProjectTagComponent.new(document: @presenter.document) %>
<%= render Show::ProjectTagComponent.new(change_set: @presenter.change_set) %>
</td>
</tr>

Expand Down
2 changes: 1 addition & 1 deletion app/components/show/item/details_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<tr>
<th class="col-3" scope="row">Project</th>
<td>
<%= render Show::ProjectTagComponent.new(document: @presenter.document) %>
<%= render Show::ProjectTagComponent.new(change_set: @presenter.change_set) %>
</td>
</tr>

Expand Down
6 changes: 6 additions & 0 deletions app/components/show/project_tag_component.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<turbo-frame id="project">
<%= project %>
<%= link_to edit_project_item_path(id: id), aria: { label: 'Edit project' } do %>
<span class="bi-pencil"></span>
<% end %>
</turbo-frame>
21 changes: 3 additions & 18 deletions app/components/show/project_tag_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,10 @@

module Show
class ProjectTagComponent < ApplicationComponent
def initialize(document:)
@document = document
def initialize(change_set:)
@change_set = change_set
end

def call
render_field 'project_tag_ssim'
end

private

delegate :blacklight_config, :search_state, :search_action_path, to: :helpers

def render_field(field_name)
field_config = fields.fetch(field_name)
Blacklight::FieldPresenter.new(self, @document, field_config).render
end

def fields
@fields ||= blacklight_config.show_fields_for(:show)
end
delegate :id, :project, to: :@change_set
end
end
7 changes: 5 additions & 2 deletions app/controllers/collections_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,13 @@ def create
def update
@cocina = maybe_load_cocina(params[:id])
authorize! :manage_item, @cocina
return unless enforce_versioning

attributes = params.require(:collection).permit(:copyright, :use_statement, :license, :project)

# Editing project does not require versioning.
return if attributes.except(:project).present? && !enforce_versioning

change_set = CollectionChangeSet.new(@cocina)
attributes = params.require(:collection).permit(:copyright, :use_statement, :license)
change_set.validate(**attributes)
change_set.save
Argo::Indexer.reindex_pid_remotely(@cocina.externalIdentifier)
Expand Down
19 changes: 16 additions & 3 deletions app/controllers/items_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class ItemsController < ApplicationController
add_collection set_collection remove_collection
mods
purge_object
show_barcode show_copyright show_license show_use_statement
show_barcode show_copyright show_license show_use_statement show_project
source_id
tags_bulk
update
Expand All @@ -20,7 +20,6 @@ class ItemsController < ApplicationController
refresh_metadata
set_rights
set_governing_apo
update
]

rescue_from Dor::Services::Client::UnexpectedResponse do |exception|
Expand Down Expand Up @@ -301,10 +300,24 @@ def show_license
render Show::LicenseComponent.new(change_set: change_set, state_service: state_service)
end

# Draw form for setting project
def edit_project
@change_set = build_change_set
end

def show_project
change_set = ItemChangeSet.new(@cocina)
render Show::ProjectTagComponent.new(change_set: change_set)
end

# save the form
def update
change_set = ItemChangeSet.new(@cocina)
attributes = params.require(:item).permit(:barcode, :copyright, :use_statement, :license)
attributes = params.require(:item).permit(:barcode, :copyright, :use_statement, :license, :project)

# Editing project does not require versioning.
return if attributes.except(:project).present? && !enforce_versioning

if change_set.validate(**attributes)
change_set.save # may raise Dor::Services::Client::BadRequestError
reindex
Expand Down
2 changes: 2 additions & 0 deletions app/models/collection_change_set.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ class CollectionChangeSet < ApplicationChangeSet
property :copyright, virtual: true
property :license, virtual: true
property :use_statement, virtual: true
property :project, virtual: true

def self.model_name
::ActiveModel::Name.new(nil, nil, 'Collection')
Expand All @@ -22,6 +23,7 @@ def setup_properties!(_options)
self.copyright = model.access.copyright
self.use_statement = model.access.useAndReproductionStatement
self.license = model.access.license
self.project = model.administrative&.partOfProject
end

def save_model
Expand Down
2 changes: 2 additions & 0 deletions app/models/item_change_set.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class ItemChangeSet < ApplicationChangeSet
property :source_id, virtual: true
property :use_statement, virtual: true
property :barcode, virtual: true
property :project, virtual: true

validates :embargo_access, inclusion: {
in: Constants::REGISTRATION_RIGHTS_OPTIONS.map(&:second),
Expand All @@ -36,6 +37,7 @@ def setup_properties!(_options)
self.copyright = model.access.copyright
self.use_statement = model.access.useAndReproductionStatement
self.license = model.access.license
self.project = model.administrative.partOfProject

setup_embargo_properties! if model.access.embargo
end
Expand Down
14 changes: 11 additions & 3 deletions app/services/collection_change_set_persister.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,24 @@ def update
updated = model
updated = update_identification(updated) if changed?(:source_id) || changed?(:catkey)
updated = updated_access(updated) if access_changed?
updated = updated_administrative(updated) if changed?(:admin_policy_id)
updated = updated_administrative(updated) if administrative_changed?
object_client.update(params: updated)
end

private

attr_reader :model, :change_set

delegate :admin_policy_id, :license, :copyright, :use_statement, :catkey, :changed?, to: :change_set
delegate :admin_policy_id, :project, :license, :copyright, :use_statement, :catkey, :changed?, to: :change_set

def access_changed?
changed?(:copyright) || changed?(:license) || changed?(:use_statement)
end

def administrative_changed?
changed?(:admin_policy_id) || changed?(:project)
end

def updated_access(updated)
access_properties = {
copyright: changed?(:copyright) ? copyright : updated.access.copyright,
Expand All @@ -49,7 +53,11 @@ def update_identification(updated)
end

def updated_administrative(updated)
updated_administrative = updated.administrative.new(hasAdminPolicy: admin_policy_id)
properties = {}
properties[:hasAdminPolicy] = admin_policy_id if changed?(:admin_policy_id)
properties[:partOfProject] = project if changed?(:project)

updated_administrative = updated.administrative.new(properties)
updated.new(administrative: updated_administrative)
end

Expand Down
14 changes: 11 additions & 3 deletions app/services/item_change_set_persister.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def update
updated = model
updated = update_structural(updated) if changed?(:collection_ids)
updated = update_identification(updated) if changed?(:source_id) || changed?(:catkey) || changed?(:barcode)
updated = updated_administrative(updated) if changed?(:admin_policy_id)
updated = updated_administrative(updated) if administrative_changed?
updated = updated_access(updated) if access_changed?
object_client.update(params: updated)
end
Expand All @@ -28,7 +28,7 @@ def update

attr_reader :model, :change_set

delegate :admin_policy_id, :barcode, :catkey, :collection_ids,
delegate :admin_policy_id, :project, :barcode, :catkey, :collection_ids,
:copyright, :embargo_release_date, :embargo_access, :license, :source_id,
:use_statement, :changed?, to: :change_set

Expand All @@ -37,7 +37,11 @@ def object_client
end

def updated_administrative(updated)
updated_administrative = updated.administrative.new(hasAdminPolicy: admin_policy_id)
properties = {}
properties[:hasAdminPolicy] = admin_policy_id if changed?(:admin_policy_id)
properties[:partOfProject] = project.presence if changed?(:project)

updated_administrative = updated.administrative.new(properties)
updated.new(administrative: updated_administrative)
end

Expand All @@ -58,6 +62,10 @@ def access_changed?
changed?(:embargo_access)
end

def administrative_changed?
changed?(:admin_policy_id) || changed?(:project)
end

def updated_access(updated)
access_properties = {}
access_properties[:copyright] = copyright.presence if changed?(:copyright)
Expand Down
12 changes: 12 additions & 0 deletions app/views/items/edit_project.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<%= turbo_frame_tag 'project' do %>
<%= form_with model: @change_set do |f| %>
<div class="mb-3">
<%= f.label :project, class: 'visually-hidden' %>
<%= f.text_field :project, class: 'form-control' %>
</div>
<div class="float-end">
<%= link_to 'Cancel', show_project_item_path(params[:id]) %>
<%= f.submit 'Save', class: 'btn btn-primary' %>
</div>
<% end %>
<% end %>
2 changes: 2 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@
get 'show_use_statement'
get 'edit_license'
get 'show_license'
get 'edit_project'
get 'show_project'
get 'collection/delete', action: :remove_collection, as: 'remove_collection'
post 'collection/add', action: :add_collection, as: 'add_collection'
post 'collection/set', action: :set_collection, as: 'set_collection'
Expand Down
2 changes: 1 addition & 1 deletion spec/components/show/collection/details_component_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
let(:presenter) { instance_double(ArgoShowPresenter, document: doc, change_set: change_set, cocina: cocina, state_service: state_service) }
let(:cocina) { instance_double(Cocina::Models::Collection) }

let(:change_set) { instance_double(ItemChangeSet, barcode: nil, id: doc.id, catkey: '') }
let(:change_set) { instance_double(CollectionChangeSet, id: doc.id, catkey: '', project: 'phoenix') }
let(:rendered) { render_inline(component) }
let(:allows_modification) { true }
let(:state_service) { instance_double(StateService, allows_modification?: allows_modification) }
Expand Down
2 changes: 1 addition & 1 deletion spec/components/show/item/details_component_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
let(:presenter) { instance_double(ArgoShowPresenter, document: doc, change_set: change_set, cocina: cocina, state_service: state_service) }
let(:cocina) { instance_double(Cocina::Models::DRO) }

let(:change_set) { instance_double(ItemChangeSet, barcode: nil, id: doc.id, catkey: '') }
let(:change_set) { instance_double(ItemChangeSet, barcode: nil, id: doc.id, catkey: '', project: 'phoenix') }
let(:rendered) { render_inline(component) }
let(:allows_modification) { true }
let(:state_service) { instance_double(StateService, allows_modification?: allows_modification) }
Expand Down
50 changes: 37 additions & 13 deletions spec/requests/collection_update_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,23 +33,47 @@
})
end

let(:updated_model) do
cocina_model.new(
{
'access' => {
'copyright' => 'in public domain'
context 'when copyright is passed' do
let(:updated_model) do
cocina_model.new(
{
'access' => {
'copyright' => 'in public domain'
}
}
}
)
)
end

it 'sets the new copyright' do
patch "/collections/#{pid}", params: { collection: { copyright: 'in public domain' } }

expect(object_client).to have_received(:update)
.with(params: updated_model)
expect(Argo::Indexer).to have_received(:reindex_pid_remotely)
expect(response.code).to eq('303')
end
end

it 'sets the new copyright' do
patch "/collections/#{pid}", params: { collection: { copyright: 'in public domain' } }
context 'when project is passed' do
let(:updated_model) do
cocina_model.new(
{
administrative: {
partOfProject: 'pegasus',
hasAdminPolicy: 'druid:cg532dg5405'
}
}
)
end

it 'sets the new project' do
patch "/collections/#{pid}", params: { collection: { project: 'pegasus' } }

expect(object_client).to have_received(:update)
.with(params: updated_model)
expect(Argo::Indexer).to have_received(:reindex_pid_remotely)
expect(response.code).to eq('303')
expect(object_client).to have_received(:update)
.with(params: updated_model)
expect(Argo::Indexer).to have_received(:reindex_pid_remotely)
expect(response.code).to eq('303')
end
end
end
end
24 changes: 23 additions & 1 deletion spec/requests/item_update_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,29 @@
end
end

context 'when there is an error building the Cocina' do
context 'when project is passed' do
let(:updated_model) do
cocina_model.new(
{
administrative: {
partOfProject: 'pegasus',
hasAdminPolicy: 'druid:cg532dg5405'
}
}
)
end

it 'sets the new project' do
patch "/items/#{pid}", params: { item: { project: 'pegasus' } }

expect(object_client).to have_received(:update)
.with(params: updated_model)
expect(Argo::Indexer).to have_received(:reindex_pid_remotely)
expect(response.code).to eq('303')
end
end

context 'when there is an error' do
it 'draws the error' do
patch "/items/#{pid}", params: { item: { barcode: 'invalid' } }, headers: { 'Turbo-Frame' => 'barcode' }

Expand Down