Replaced People.wikidata_id with wikidata_uri

app/controllers/authors_controller.rb

@@ -380,7 +380,6 @@ def new
   end
 
   def create
-    params[:person][:wikidata_id] = params[:person][:wikidata_id].strip[1..-1] if params[:person] and params[:person][:wikidata_id] and params[:person][:wikidata_id][0] and params[:person][:wikidata_id].strip[0] == 'Q' # tolerate pasting the Wikidata number with the Q
     Chewy.strategy(:atomic) {
       @person = Person.new(person_params)
       if @person.status.blank?
@@ -423,7 +422,6 @@ def edit
   def update
     @author = Person.find(params[:id])
 
-    params[:person][:wikidata_id] = params[:person][:wikidata_id].strip[1..-1] if params[:person] and params[:person][:wikidata_id] and params[:person][:wikidata_id][0] and params[:person][:wikidata_id].strip[0] == 'Q' # tolerate pasting the Wikidata number with the Q
     Chewy.strategy(:atomic) do
       if @author.update(person_params)
         # if period was updated, update the period of this person's Expressions
@@ -440,8 +438,7 @@ def update
         flash[:notice] = I18n.t(:updated_successfully)
         redirect_to action: :show, id: @author.id
       else
-        format.html { render action: 'edit' }
-        format.json { render json: @author.errors, status: :unprocessable_entity }
+        render action: 'edit', status: :unprocessable_entity
       end
     end
   end
@@ -590,7 +587,7 @@ def person_params
       :profile_image,
       :birthdate,
       :deathdate,
-      :wikidata_id,
+      :wikidata_uri,
       :wikipedia_url,
       :wikipedia_snippet,
       :blog_category_url,

app/models/person.rb

@@ -1,5 +1,6 @@
 include BybeUtils
 class Person < ApplicationRecord
+  WIKIDATA_URI_PATTERN = %r{\Ahttps://wikidata.org/wiki/q[0-9]+\z}
 
   enum gender: %i(male female other unknown)
   enum period: %i(ancient medieval enlightenment revival modern)
@@ -48,11 +49,16 @@ class Person < ApplicationRecord
 
   # validations
   validates :name, :intellectual_property, presence: true
+  validates :wikidata_uri, format: WIKIDATA_URI_PATTERN, allow_nil: true
 
   validates_attachment_content_type :profile_image, content_type: /\Aimage\/.*\z/
 
   update_index('people'){self} # update PeopleIndex when entity is updated
 
+  before_validation do
+    self.wikidata_uri = wikidata_uri.blank? ? nil : wikidata_uri.strip.downcase
+  end
+
   # class variable
   @@popular_authors = nil
 

app/views/authors/_form.html.haml

@@ -66,8 +66,8 @@
     = f.label t(:blog_category_url)
     = f.text_area :blog_category_url, rows: 3, cols: 50
   .backend-field
-    = f.label t(:wikidata_id)
-    = f.text_field :wikidata_id, size: 10
+    = f.label Person.human_attribute_name(:wikidata_uri)
+    = f.text_field :wikidata_uri
   .backend-field
     = t(:wikipedia_url)
     = f.text_area :wikipedia_url, rows: 3, cols: 50

app/views/authors/show.html.haml

@@ -72,9 +72,9 @@
         - unless(@author.blog_category_url.nil? or @author.blog_category_url.empty?)
           %a{ href: @author.blog_category_url}= t(:link)
         %br
-        %b= t(:wikidata_id)
-        - unless @author.wikidata_id.nil?
-          %a{ href: "https://wikidata.org/wiki/Q#{@author.wikidata_id}"}= @author.wikidata_id.to_s
+        %b= Person.human_attribute_name(:wikidata_uri)
+        - if @author.wikidata_uri.present?
+          = link_to @author.wikidata_uri, @author.wikidata_uri
         %br
         %b= t(:wikipedia_url)
         - unless @author.wikipedia_url.nil? or @author.wikipedia_url.empty?

config/locales/active_record.en.yml

@@ -18,3 +18,4 @@ en:
         intellectual_property: Type of intellectual property
       person:
         intellectual_property: Type of intellectual property
+        wikidata_uri: Wikidata URI

config/locales/active_record.he.yml

@@ -18,3 +18,4 @@ he:
         intellectual_property: מצב זכויות יוצרים
       person:
         intellectual_property: מצב זכויות יוצרים
+        wikidata_uri: Wikidata URI

config/locales/he.yml

@@ -409,7 +409,6 @@ he:
   rest_in_wikipedia: להמשך הערך בויקיפדיה
   rest_of_work: להמשך קריאה
   to_the_wikipedia_article: לערך בויקיפדיה
-  wikidata_id: מזהה בויקינתונים
   additional_options: אפשרויות נוספות
   raw: טרם עוּבַּד
   ready: מוכן

db/migrate/20240516170503_change_people_wikidata_id_to_wikidata_uri.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class ChangePeopleWikidataIdToWikidataUri < ActiveRecord::Migration[6.1]
+  def change
+    add_column :people, :wikidata_uri, :string
+
+    execute <<~SQL
+      update people
+      set wikidata_uri = CONCAT('https://wikidata.org/wiki/q', wikidata_id)
+      where wikidata_id is not null
+    SQL
+
+    remove_column :people, :wikidata_id
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2024_05_15_114242) do
+ActiveRecord::Schema.define(version: 2024_05_16_170503) do
 
   create_table "aboutnesses", id: :integer, charset: "utf8mb4", collation: "utf8mb4_bin", force: :cascade do |t|
     t.integer "work_id"
@@ -571,7 +571,6 @@
     t.string "profile_image_content_type"
     t.integer "profile_image_file_size"
     t.datetime "profile_image_updated_at"
-    t.integer "wikidata_id"
     t.string "birthdate"
     t.string "deathdate"
     t.integer "gender"
@@ -583,6 +582,7 @@
     t.integer "status"
     t.datetime "published_at"
     t.integer "intellectual_property", null: false
+    t.string "wikidata_uri"
     t.index ["gender"], name: "gender_index"
     t.index ["impressions_count"], name: "index_people_on_impressions_count"
     t.index ["intellectual_property"], name: "index_people_on_intellectual_property"

spec/controllers/authors_controller_spec.rb

@@ -256,13 +256,15 @@
             person: {
               name: new_name,
               period: new_period,
-              intellectual_property: new_intellectual_property
+              intellectual_property: new_intellectual_property,
+              wikidata_uri: new_wikidata_uri
             }
           }
         end
 
         let(:new_name) { 'New Name' }
         let(:new_intellectual_property) { 'unknown' }
+        let(:new_wikidata_uri) { 'https://wikidata.org/wiki/Q1234' }
 
         let(:works_period) { 'modern' } # intentionally use value different from author period
         let!(:original_work) { create(:manifestation, orig_lang: 'he', author: author, period: works_period) }
@@ -279,7 +281,8 @@
             expect(author).to have_attributes(
               name: new_name,
               period: new_period,
-              intellectual_property: new_intellectual_property
+              intellectual_property: new_intellectual_property,
+              wikidata_uri: new_wikidata_uri
             )
             original_work.reload
             expect(original_work.expression.period).to eq new_period
@@ -305,7 +308,8 @@
             expect(author).to have_attributes(
               name: new_name,
               period: new_period,
-              intellectual_property: new_intellectual_property
+              intellectual_property: new_intellectual_property,
+              wikidata_uri: new_wikidata_uri
             )
             original_work.reload
             expect(original_work.expression.period).to eq works_period
@@ -317,6 +321,16 @@
             expect(translated_to_foreign_work.expression.period).to eq works_period
           end
         end
+
+        context 'when validation error occurs' do
+          let(:new_wikidata_uri) { 'https://wrongsite.com/Q123' }
+          let(:new_period) { period }
+
+          it 'fails to save and re-renders edit form' do
+            expect(request).to have_http_status(:unprocessable_entity)
+            expect(response).to render_template :edit
+          end
+        end
       end
     end
   end

spec/models/person_spec.rb

@@ -13,6 +13,42 @@
       p = described_class.new(name: Faker::Artist.name, intellectual_property: :public_domain)
       expect(p).to be_valid
     end
+
+    describe '.wikidata_uri' do
+      subject(:result) { person.valid? }
+
+      let(:person) { build(:person, wikidata_uri: value) }
+
+      context 'when value is blank' do
+        let(:value) { '  ' }
+
+        it 'succeed but sets value to nil' do
+          expect(result).to be_truthy
+          expect(person.wikidata_uri).to be_nil
+        end
+      end
+
+      context 'when uri has wrong format' do
+        let(:value) { 'http://wikidata.org/wiki/q1234' }
+
+        it { is_expected.to be false }
+      end
+
+      context 'when uri is correct but is is not numeric' do
+        let(:value) { 'https://wikidata.org/wiki/q1234A' }
+
+        it { is_expected.to be false }
+      end
+
+      context 'when value has correct format' do
+        let(:value) { '  HTTPS://wikidata.org/WIKI/Q1234  ' }
+
+        it 'succeed, removes leading/trailing whitespaces and converts to downcase' do
+          expect(result).to be true
+          expect(person.wikidata_uri).to eq 'https://wikidata.org/wiki/q1234'
+        end
+      end
+    end
   end
 
   describe 'instance methods' do