Skip to content

Commit

Permalink
use role_name #697
Browse files Browse the repository at this point in the history
  • Loading branch information
Martin Fenner committed Jan 29, 2021
1 parent 6eeab13 commit 0889833
Show file tree
Hide file tree
Showing 9 changed files with 60 additions and 75 deletions.
5 changes: 3 additions & 2 deletions app/controllers/contacts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/exports_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
39 changes: 21 additions & 18 deletions app/models/contact.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,15 @@ 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
validates_format_of :email,
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?
Expand Down Expand Up @@ -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
Expand All @@ -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),
Expand All @@ -92,19 +89,25 @@ def self.query_fields
family_name^10
name^5
email^10
roles^10
role_name^10
_all
]
end

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
Expand All @@ -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}."
Expand All @@ -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}."
Expand All @@ -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}."
Expand All @@ -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}."
Expand All @@ -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}."
Expand All @@ -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}."
Expand All @@ -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}."
Expand Down
3 changes: 1 addition & 2 deletions app/models/provider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
2 changes: 1 addition & 1 deletion app/serializers/contact_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class ContactSerializer
:family_name,
:name,
:email,
:roles,
:role_name,
:created,
:updated,
:deleted
Expand Down
2 changes: 1 addition & 1 deletion db/migrate/20210118095023_add_contact_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
26 changes: 5 additions & 21 deletions db/schema.rb
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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|
Expand Down Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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

Expand All @@ -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
2 changes: 1 addition & 1 deletion spec/factories/default.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
email { "[email protected]" }
given_name { "Josiah" }
family_name { "Carberry" }
roles { ["voting"] }
role_name { ["voting"] }

initialize_with { Contact.where(uid: uid).first_or_initialize }
end
Expand Down
54 changes: 26 additions & 28 deletions spec/requests/contacts_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"givenName" => "Josiah",
"familyName" => "Carberry",
"email" => "[email protected]",
"roles" => ["voting_contact"]
"roleName" => ["voting"]
},
"relationships": {
"provider": {
Expand Down Expand Up @@ -90,7 +90,7 @@
attributes = json.dig("data", "attributes")
expect(attributes["name"]).to eq("Josiah Carberry")
expect(attributes["email"]).to eq("[email protected]")
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" } })
Expand All @@ -106,7 +106,7 @@
attributes = json.dig("data", 0, "attributes")
expect(attributes["name"]).to eq("Josiah Carberry")
expect(attributes["email"]).to eq("[email protected]")
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(
Expand Down Expand Up @@ -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"],
},
},
}
Expand All @@ -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
Expand Down

0 comments on commit 0889833

Please sign in to comment.