From 6342e90acaa511bad7a134035a8a3e868d81d18b Mon Sep 17 00:00:00 2001 From: Damir Sultanbekov Date: Thu, 16 May 2024 23:30:19 +0500 Subject: [PATCH] Replaced People.wikidata_id with wikidata_uri --- app/controllers/authors_controller.rb | 7 ++-- app/models/person.rb | 6 ++++ app/views/authors/_form.html.haml | 4 +-- app/views/authors/show.html.haml | 6 ++-- config/locales/active_record.en.yml | 1 + config/locales/active_record.he.yml | 1 + config/locales/he.yml | 1 - ...ange_people_wikidata_id_to_wikidata_uri.rb | 15 ++++++++ db/schema.rb | 4 +-- spec/controllers/authors_controller_spec.rb | 20 +++++++++-- spec/models/person_spec.rb | 36 +++++++++++++++++++ 11 files changed, 85 insertions(+), 16 deletions(-) create mode 100644 db/migrate/20240516170503_change_people_wikidata_id_to_wikidata_uri.rb diff --git a/app/controllers/authors_controller.rb b/app/controllers/authors_controller.rb index 9dee7b97..1ccdd9a2 100644 --- a/app/controllers/authors_controller.rb +++ b/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, diff --git a/app/models/person.rb b/app/models/person.rb index e0ec3adb..9fa7065a 100644 --- a/app/models/person.rb +++ b/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 diff --git a/app/views/authors/_form.html.haml b/app/views/authors/_form.html.haml index 00e99bde..47aa4d9e 100644 --- a/app/views/authors/_form.html.haml +++ b/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 diff --git a/app/views/authors/show.html.haml b/app/views/authors/show.html.haml index fa352674..da2cdbdd 100644 --- a/app/views/authors/show.html.haml +++ b/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? diff --git a/config/locales/active_record.en.yml b/config/locales/active_record.en.yml index 2441704b..7cbd29c8 100644 --- a/config/locales/active_record.en.yml +++ b/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 diff --git a/config/locales/active_record.he.yml b/config/locales/active_record.he.yml index ba6a051e..de250fe9 100644 --- a/config/locales/active_record.he.yml +++ b/config/locales/active_record.he.yml @@ -18,3 +18,4 @@ he: intellectual_property: מצב זכויות יוצרים person: intellectual_property: מצב זכויות יוצרים + wikidata_uri: Wikidata URI diff --git a/config/locales/he.yml b/config/locales/he.yml index d5d8b2a8..f0668f58 100644 --- a/config/locales/he.yml +++ b/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: מוכן diff --git a/db/migrate/20240516170503_change_people_wikidata_id_to_wikidata_uri.rb b/db/migrate/20240516170503_change_people_wikidata_id_to_wikidata_uri.rb new file mode 100644 index 00000000..5a516cfb --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 76ef7161..3f4a6982 100755 --- a/db/schema.rb +++ b/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" diff --git a/spec/controllers/authors_controller_spec.rb b/spec/controllers/authors_controller_spec.rb index fc1a6a8f..18ed3d02 100644 --- a/spec/controllers/authors_controller_spec.rb +++ b/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 diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb index 56829e07..82d47cbf 100644 --- a/spec/models/person_spec.rb +++ b/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