Skip to content

Commit

Permalink
Making work.metadata protected
Browse files Browse the repository at this point in the history
This will keep folks from assigning directly to the metadata field
fixes #277
  • Loading branch information
carolyncole committed Aug 29, 2022
1 parent ff5c9e7 commit 8365723
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 13 deletions.
4 changes: 2 additions & 2 deletions app/controllers/works_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def new
def new_submission
default_collection_id = current_user.default_collection.id
resource = resource_from_form
work = Work.new(created_by_user_id: current_user.id, collection_id: default_collection_id, metadata: resource.to_json)
work = Work.new(created_by_user_id: current_user.id, collection_id: default_collection_id, resource: resource)
work.draft!(current_user)
redirect_to edit_work_path(work, wizard: true)
end
Expand Down Expand Up @@ -79,7 +79,7 @@ def update

updates = {
collection_id: collection_id_param,
metadata: resource_from_form.to_json,
resource: resource_from_form,
pre_curation_uploads: updated_pre_curation_uploads
}

Expand Down
14 changes: 14 additions & 0 deletions app/models/work.rb
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,11 @@ def created_by_user
nil
end

def resource=(resource)
@resource = resource
self.metadata = resource.to_json
end

def resource
@resource ||= PDCMetadata::Resource.new_from_json(metadata)
end
Expand Down Expand Up @@ -324,6 +329,15 @@ def add_post_curation_uploads(s3_file)
post_curation_uploads << ActiveStorage::Attachment.new(blob: blob, name: :post_curation_uploads)
end

protected

# This must be protected, NOT private for AcrtiveRecord to work properly with this attribute.
# Protected will still keep others from setting the metatdata, but allows ActiveRecord the access it needs
def metadata=(lmetadata)
super
@resource = PDCMetadata::Resource.new_from_json(metadata)
end

private

def publish(user)
Expand Down
9 changes: 4 additions & 5 deletions spec/factories/work.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,17 @@
factory :draft_work do
transient do
doi { "https://doi.org/10.34770/123-abc" }
resource { FactoryBot.build :resource, doi: doi }
end
collection { Collection.research_data }
state { "draft" }
created_by_user_id { FactoryBot.create(:user).id }
metadata { resource.to_json }
resource { FactoryBot.build :resource, doi: doi }
end

factory :shakespeare_and_company_work do
collection { Collection.research_data }
metadata do
{
resource do
PDCMetadata::Resource.new_from_json({
"doi": "https://doi.org/10.34770/pe9w-x904",
"ark": "ark:/88435/dsp01zc77st047",
"identifier_type": "DOI",
Expand All @@ -26,7 +25,7 @@
{ "value": "Kotin, Joshua", "name_type": "Personal", "given_name": "Joshua", "family_name": "Kotin", "affiliations": [], "sequence": "1" }
],
"resource_type": "Dataset", "publisher": "Princeton University", "publication_year": "2020"
}.to_json
}.to_json)
end
created_by_user_id { FactoryBot.create(:user).id }
end
Expand Down
10 changes: 5 additions & 5 deletions spec/models/work_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,19 @@
end

it "drafts a doi only once" do
work = Work.new(collection: collection, metadata: FactoryBot.build(:resource).to_json)
work = Work.new(collection: collection, resource: FactoryBot.build(:resource))
work.draft_doi
work.draft_doi # Doing this multiple times on purpose to make sure the api is only called once
expect(a_request(:post, ENV["DATACITE_URL"])).to have_been_made.once
end

it "prevents datasets with no users" do
work = Work.new(collection: collection, metadata: PDCMetadata::Resource.new.to_json)
work = Work.new(collection: collection, resource: PDCMetadata::Resource.new)
expect { work.draft! }.to raise_error AASM::InvalidTransition
end

it "prevents datasets with no collections" do
work = Work.new(collection: nil, metadata: FactoryBot.build(:resource).to_json)
work = Work.new(collection: nil, resource: FactoryBot.build(:resource))
expect { work.save! }.to raise_error ActiveRecord::RecordInvalid
end

Expand Down Expand Up @@ -327,7 +327,7 @@

describe "#draft" do
let(:draft_work) do
work = Work.new(collection: collection, metadata: FactoryBot.build(:resource).to_json)
work = Work.new(collection: collection, resource: FactoryBot.build(:resource))
work.draft!(user)
work = Work.find(work.id)
work
Expand Down Expand Up @@ -494,7 +494,7 @@
end

describe "states" do
let(:work) { Work.new(collection: collection, metadata: FactoryBot.build(:resource).to_json) }
let(:work) { Work.new(collection: collection, resource: FactoryBot.build(:resource)) }
it "initally is none" do
expect(work.none?).to be_truthy
expect(work.state).to eq("none")
Expand Down
2 changes: 1 addition & 1 deletion spec/system/work_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
it "Renders ORCID links for creators", js: true do
stub_s3
resource = FactoryBot.build(:resource, creators: [PDCMetadata::Creator.new_person("Harriet", "Tubman", "1234-5678-9012-3456")])
work = FactoryBot.create(:draft_work, metadata: resource.to_json)
work = FactoryBot.create(:draft_work, resource: resource)

sign_in user
visit work_path(work)
Expand Down

0 comments on commit 8365723

Please sign in to comment.