From 0889833e451f0f08929967cee1e1ebf88a50699a Mon Sep 17 00:00:00 2001 From: Martin Fenner Date: Fri, 29 Jan 2021 07:49:16 +0100 Subject: [PATCH] use role_name #697 --- app/controllers/contacts_controller.rb | 5 +- app/controllers/exports_controller.rb | 2 +- app/models/contact.rb | 39 +++++++------- app/models/provider.rb | 3 +- app/serializers/contact_serializer.rb | 2 +- .../20210118095023_add_contact_model.rb | 2 +- db/schema.rb | 26 ++------- spec/factories/default.rb | 2 +- spec/requests/contacts_spec.rb | 54 +++++++++---------- 9 files changed, 60 insertions(+), 75 deletions(-) diff --git a/app/controllers/contacts_controller.rb b/app/controllers/contacts_controller.rb index dbf4f098f..9f8f1be3e 100644 --- a/app/controllers/contacts_controller.rb +++ b/app/controllers/contacts_controller.rb @@ -176,13 +176,14 @@ def safe_params :givenName, :familyName, :email, - :roles, - { roles: [] }, + :roleName, + { roleName: [] }, :provider ], keys: { "givenName" => :given_name, "familyName" => :family_name, + "roleName" => :role_name, }, ) end diff --git a/app/controllers/exports_controller.rb b/app/controllers/exports_controller.rb index fd6064c20..813448f90 100644 --- a/app/controllers/exports_controller.rb +++ b/app/controllers/exports_controller.rb @@ -31,7 +31,7 @@ def contacts "email" => contact.email, "firstName" => contact.given_name, "lastName" => contact.family_name, - "type" => contact.roles.join(";"), + "type" => contact.role_name.join(";"), "createdAt" => contact.created_at.try(:iso8601), "modifiedAt" => contact.updated_at.try(:iso8601), "deletedAt" => contact.deleted_at.try(:iso8601), diff --git a/app/models/contact.rb b/app/models/contact.rb index ae9fa92b1..043121938 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -15,11 +15,7 @@ class Contact < ApplicationRecord before_create :set_uid - ROLES = %w[voting_contact billing_contact secondary_billing_contact service_contact secondary_service_contact technical_contact secondary_technical_contact] - - # validates_inclusion_of :role_name, - # in: ROLES, - # message: "Role %s is not included in the list of possible roles" + ROLES = %w[voting billing secondary_billing service secondary_service technical secondary_technical] validates_presence_of :provider validates_presence_of :email @@ -27,6 +23,7 @@ class Contact < ApplicationRecord with: /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/i, message: "email should be valid", allow_blank: true + validate :check_role_name, if: :role_name? # use different index for testing if Rails.env.test? @@ -57,12 +54,12 @@ class Contact < ApplicationRecord mapping dynamic: "false" do indexes :id, type: :keyword indexes :uid, type: :keyword, normalizer: "keyword_lowercase" - indexes :provider_id, type: :keyword + indexes :provider_id, type: :keyword, normalizer: "keyword_lowercase" indexes :given_name, type: :keyword, normalizer: "keyword_lowercase" indexes :family_name, type: :keyword, normalizer: "keyword_lowercase" indexes :name, type: :keyword, normalizer: "keyword_lowercase" - indexes :email, type: :keyword - indexes :roles, type: :keyword + indexes :email, type: :keyword, normalizer: "keyword_lowercase" + indexes :role_name, type: :keyword, normalizer: "keyword_lowercase" indexes :created_at, type: :date indexes :updated_at, type: :date indexes :deleted_at, type: :date @@ -77,7 +74,7 @@ def as_indexed_json(options = {}) "family_name" => family_name, "name" => name, "email" => email, - "roles" => roles, + "role_name" => role_name, "provider_id" => provider_id, "created_at" => created_at.try(:iso8601), "updated_at" => updated_at.try(:iso8601), @@ -92,7 +89,7 @@ def self.query_fields family_name^10 name^5 email^10 - roles^10 + role_name^10 _all ] end @@ -100,11 +97,17 @@ def self.query_fields def self.query_aggregations { roles: { - terms: { field: "roles", size: 10, min_doc_count: 1 }, + terms: { field: "role_name", size: 10, min_doc_count: 1 }, }, } end + def check_role_name + Array.wrap(role_name).each do |r| + errors.add(:role_name, "Role name '#{r}' is not included in the list of possible role names.") unless ROLES.include?(r) + end + end + def name [given_name, family_name].compact.join(" ") end @@ -130,7 +133,7 @@ def self.import_from_providers provider_id: provider.uid, given_name: provider.voting_contact_given_name, family_name: provider.voting_contact_family_name, - roles: (Array.wrap(contact.roles) << "voting").uniq) + role_name: (Array.wrap(contact.role_name) << "voting").uniq) puts "Imported voting contact #{contact.email} for provider #{provider.symbol}." else puts "Error importing voting contact #{contact.email} for provider #{provider.symbol}: #{contact.errors.messages.inspect}." @@ -143,7 +146,7 @@ def self.import_from_providers provider_id: provider.uid, given_name: provider.billing_contact_given_name, family_name: provider.billing_contact_family_name, - roles: (Array.wrap(contact.roles) << "billing").uniq) + role_name: (Array.wrap(contact.role_name) << "billing").uniq) puts "Imported billing contact #{contact.email} for provider #{provider.symbol}." else puts "Error importing billing contact #{contact.email} for provider #{provider.symbol}: #{contact.errors.messages.inspect}." @@ -156,7 +159,7 @@ def self.import_from_providers provider_id: provider.uid, given_name: provider.secondary_billing_contact_given_name, family_name: provider.secondary_billing_contact_family_name, - roles: (Array.wrap(contact.roles) << "secondaryBilling").uniq) + role_name: (Array.wrap(contact.role_name) << "secondaryBilling").uniq) puts "Imported secondary billing contact #{contact.email} for provider #{provider.symbol}." else puts "Error importing secondary technical contact #{contact.email} for provider #{provider.symbol}: #{contact.errors.messages.inspect}." @@ -169,7 +172,7 @@ def self.import_from_providers provider_id: provider.uid, given_name: provider.service_contact_given_name, family_name: provider.service_contact_family_name, - roles: (Array.wrap(contact.roles) << "service").uniq) + role_name: (Array.wrap(contact.role_name) << "service").uniq) puts "Imported service contact #{contact.email} for provider #{provider.symbol}." else puts "Error importing service contact #{contact.email} for provider #{provider.symbol}: #{contact.errors.messages.inspect}." @@ -182,7 +185,7 @@ def self.import_from_providers provider_id: provider.uid, given_name: provider.secondary_service_contact_given_name, family_name: provider.secondary_service_contact_family_name, - roles: (Array.wrap(contact.roles) << "secondaryService").uniq) + role_name: (Array.wrap(contact.role_name) << "secondaryService").uniq) puts "Imported secondary service contact #{contact.email} for provider #{provider.symbol}." else puts "Error importing secondary service contact #{contact.email} for provider #{provider.symbol}: #{contact.errors.messages.inspect}." @@ -195,7 +198,7 @@ def self.import_from_providers provider_id: provider.uid, given_name: provider.technical_contact_given_name, family_name: provider.technical_contact_family_name, - roles: (Array.wrap(contact.roles) << "technical").uniq) + role_name: (Array.wrap(contact.role_name) << "technical").uniq) puts "Imported technical contact #{contact.email} for provider #{provider.symbol}." else puts "Error importing technical contact #{contact.email} for provider #{provider.symbol}: #{contact.errors.messages.inspect}." @@ -208,7 +211,7 @@ def self.import_from_providers provider_id: provider.uid, given_name: provider.secondary_technical_contact_given_name, family_name: provider.secondary_technical_contact_family_name, - roles: (Array.wrap(contact.roles) << "secondaryTechnical").uniq) + role_name: (Array.wrap(contact.role_name) << "secondaryTechnical").uniq) puts "Imported secondary technical contact #{contact.email} for provider #{provider.symbol}." else puts "Error importing secondary technical contact #{contact.email} for provider #{provider.symbol}: #{contact.errors.messages.inspect}." diff --git a/app/models/provider.rb b/app/models/provider.rb index 3c5620cb2..85e0ed2c8 100644 --- a/app/models/provider.rb +++ b/app/models/provider.rb @@ -164,8 +164,7 @@ class Provider < ApplicationRecord inverse_of: :consortium_organizations, optional: true has_many :activities, as: :auditable, dependent: :destroy - has_many :roles, dependent: :destroy - has_many :contacts, through: :roles + has_many :contacts before_validation :set_region, :set_defaults before_create { self.created = Time.zone.now.utc.iso8601 } diff --git a/app/serializers/contact_serializer.rb b/app/serializers/contact_serializer.rb index f62013066..ec20dd407 100644 --- a/app/serializers/contact_serializer.rb +++ b/app/serializers/contact_serializer.rb @@ -10,7 +10,7 @@ class ContactSerializer :family_name, :name, :email, - :roles, + :role_name, :created, :updated, :deleted diff --git a/db/migrate/20210118095023_add_contact_model.rb b/db/migrate/20210118095023_add_contact_model.rb index f427de202..86bbe9f43 100644 --- a/db/migrate/20210118095023_add_contact_model.rb +++ b/db/migrate/20210118095023_add_contact_model.rb @@ -8,7 +8,7 @@ def change t.string "given_name" t.string "family_name" t.string "email" - t.json "roles" + t.json "role_name" t.datetime "created_at" t.datetime "updated_at" t.datetime "deleted_at" diff --git a/db/schema.rb b/db/schema.rb index d3e672121..6a42e3c83 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1,5 +1,3 @@ -# frozen_string_literal: true - # This file is auto-generated from the current state of the database. Instead # of editing this file, please use the migrations feature of Active Record to # incrementally modify your database, and then regenerate this schema definition. @@ -13,6 +11,7 @@ # It's strongly recommended that you check this file into your version control system. ActiveRecord::Schema.define(version: 2021_01_18_095023) do + create_table "active_storage_attachments", options: "ENGINE=InnoDB DEFAULT CHARSET=latin1", force: :cascade do |t| t.string "name", limit: 191, null: false t.string "record_type", null: false @@ -77,11 +76,9 @@ t.string "logo_content_type" t.bigint "logo_file_size" t.datetime "logo_updated_at" - t.string "uid", limit: 32 t.index ["globus_uuid"], name: "index_allocator_on_globus_uuid" t.index ["organization_type"], name: "index_allocator_organization_type" t.index ["symbol"], name: "symbol", unique: true - t.index ["uid"], name: "index_allocator_on_uid" end create_table "audits", options: "ENGINE=InnoDB DEFAULT CHARSET=latin1", force: :cascade do |t| @@ -112,8 +109,7 @@ t.datetime "created_at" t.datetime "updated_at" t.bigint "provider_prefix_id" - t.string "uid" - t.index ["client_id", "prefix_id"], name: "index_client_prefixes_on_client_id_and_prefix_id", unique: true + t.string "uid", null: false t.index ["client_id"], name: "FK13A1B3BA47B5F5FF" t.index ["prefix_id"], name: "FK13A1B3BAAF86A1C7" t.index ["provider_prefix_id"], name: "index_client_prefixes_on_provider_prefix_id" @@ -126,7 +122,7 @@ t.string "given_name" t.string "family_name" t.string "email" - t.json "roles" + t.json "role_name" t.datetime "created_at" t.datetime "updated_at" t.datetime "deleted_at" @@ -163,12 +159,10 @@ t.string "salesforce_id", limit: 191 t.json "service_contact" t.string "globus_uuid", limit: 191 - t.string "uid", limit: 32 t.index ["allocator"], name: "FK6695D60546EBD781" t.index ["globus_uuid"], name: "index_datacentre_on_globus_uuid" t.index ["re3data_id"], name: "index_datacentre_on_re3data_id" t.index ["symbol"], name: "symbol", unique: true - t.index ["uid"], name: "index_datacentre_on_uid" t.index ["url"], name: "index_datacentre_on_url", length: 100 end @@ -229,7 +223,7 @@ t.index ["url"], name: "index_dataset_on_url", length: 100 end - create_table "events", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin", force: :cascade do |t| + create_table "events", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4", force: :cascade do |t| t.text "uuid", null: false t.text "subj_id", null: false t.text "obj_id" @@ -289,7 +283,6 @@ create_table "prefixes", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| t.datetime "created_at" t.string "uid", limit: 80, null: false - t.string "ra", default: "DataCite" t.index ["uid"], name: "prefix", unique: true end @@ -298,19 +291,10 @@ t.bigint "prefix_id", null: false t.datetime "created_at" t.datetime "updated_at" - t.string "uuid" - t.string "uid" + t.string "uid", null: false t.index ["prefix_id"], name: "FKE7FBD674AF86A1C7" - t.index ["provider_id", "prefix_id"], name: "index_provider_prefixes_on_provider_id_and_prefix_id", unique: true t.index ["provider_id"], name: "FKE7FBD67446EBD781" t.index ["uid"], name: "index_provider_prefixes_on_uid", length: 128 end - add_foreign_key "client_prefixes", "datacentre", column: "client_id", name: "_FK13A1B3BA47B5F5FF" - add_foreign_key "client_prefixes", "prefixes", name: "FK13A1B3BAAF86A1C7" - add_foreign_key "datacentre", "allocator", column: "allocator", name: "_FK6695D60546EBD781" - add_foreign_key "media", "dataset", column: "dataset", name: "FK62F6FE44D3D6B1B" - add_foreign_key "metadata", "dataset", column: "dataset", name: "FKE52D7B2F4D3D6B1B" - add_foreign_key "provider_prefixes", "allocator", column: "provider_id", name: "FKE7FBD67446EBD781" - add_foreign_key "provider_prefixes", "prefixes", name: "FKE7FBD674AF86A1C7" end diff --git a/spec/factories/default.rb b/spec/factories/default.rb index 88a9d9b7c..d0e60e4db 100644 --- a/spec/factories/default.rb +++ b/spec/factories/default.rb @@ -76,7 +76,7 @@ email { "josiah@example.org" } given_name { "Josiah" } family_name { "Carberry" } - roles { ["voting"] } + role_name { ["voting"] } initialize_with { Contact.where(uid: uid).first_or_initialize } end diff --git a/spec/requests/contacts_spec.rb b/spec/requests/contacts_spec.rb index 19e9cb679..ea5d7d7fd 100644 --- a/spec/requests/contacts_spec.rb +++ b/spec/requests/contacts_spec.rb @@ -14,7 +14,7 @@ "givenName" => "Josiah", "familyName" => "Carberry", "email" => "bob@example.com", - "roles" => ["voting_contact"] + "roleName" => ["voting"] }, "relationships": { "provider": { @@ -90,7 +90,7 @@ attributes = json.dig("data", "attributes") expect(attributes["name"]).to eq("Josiah Carberry") expect(attributes["email"]).to eq("bob@example.com") - expect(attributes["roles"]).to eq(["voting_contact"]) + expect(attributes["roleName"]).to eq(["voting"]) relationships = json.dig("data", "relationships") expect(relationships).to eq("provider" => { "data" => { "id" => provider.uid, "type" => "providers" } }) @@ -106,7 +106,7 @@ attributes = json.dig("data", 0, "attributes") expect(attributes["name"]).to eq("Josiah Carberry") expect(attributes["email"]).to eq("josiah@example.org") - expect(attributes["roles"]).to eq(["voting_contact"]) + expect(attributes["roleName"]).to eq(["voting"]) relationships = json.dig("data", 0, "relationships") expect(relationships.dig("provider", "data", "id")).to eq( @@ -172,13 +172,13 @@ end end - context "updates roles" do + context "updates role_name" do let(:params) do { "data" => { "type" => "contacts", "attributes" => { - "roles" => ["technical_contact", "service_contact"], + "roleName" => ["technical", "service"], }, }, } @@ -188,33 +188,31 @@ put "/contacts/#{contact.uid}", params, headers expect(last_response.status).to eq(200) - expect(json.dig("data", "attributes", "roles")).to eq( - ["technical_contact", "service_contact"], + expect(json.dig("data", "attributes", "roleName")).to eq( + ["technical", "service"], ) end end - # context "updates roles invalid" do - # let(:params) do - # { - # "data" => { - # "type" => "contacts", - # "attributes" => { - # "roles" => ["technical_contact", "service_contact"], - # }, - # }, - # } - # end - - # it "updates the record" do - # put "/contacts/#{contact.uid}", params, headers - - # expect(last_response.status).to eq(200) - # expect(json.dig("data", "attributes", "roles")).to eq( - # ["technical_contact", "service_contact"], - # ) - # end - # end + context "updates role_name invalid" do + let(:params) do + { + "data" => { + "type" => "contacts", + "attributes" => { + "roleName" => ["catering"], + }, + }, + } + end + + it "updates the record" do + put "/contacts/#{contact.uid}", params, headers + + expect(last_response.status).to eq(422) + expect(json["errors"]).to eq([{ "source" => "role_name", "title" => "Role name 'catering' is not included in the list of possible role names.", "uid" => contact.uid }]) + end + end context "removes given name and family name" do