Kentucky topics

[cokernel]

This relates to #36.

[cokernel]

@rrmait0, this is incomplete. The models and the database schema still indicate a many-to-one relationship between records and Kentucky topics.

[rrmait0]

@cokernel Is it suppose to be many-to-many relationship between records and Kentucky topics?

[cokernel]

Yes. See #36. Each record needs to be able to include zero or more Kentucky topics. This requires a many-to-many relationship between their respective tables.

[rrmait0]

Okay, I'll figure it out. There's some work that needs to be done with schema.rb. The kentucky_topic_id field from the records table needs to be dropped. The kentucky_topic_id index needs to be removed. A new table has to be created called kentucky_topics_records with the following fields of kentucky_topic_id and record_id. Also, new indexes similar to languages_records has to be created.

[cokernel]

I don't recommend editing schema.rb directly. If you generate and run appropriate migrations of the type you describe in the rest of your comment, schema.rb should automatically be repaired.

[rrmait0]

I'm not editing the schema.rb directly. I'm going to generate a rail migration.

[rrmait0]

rails g CreateKentuckyTopicRecords Table which should create the following below:

class CreateKentucktyTopicsRecordsTable <ActiveRecord::Migration
         def self.up
           create_table :kentucky_topic_records, :id => false do |t|
           t.reference :kentucky_topic
           t.reference :records
         end
           add_index :kentucky_topics_records, [:kentucky_topic_id, record_id]
           add_index :kentucky_topics_records, :record_id
         end

         def self.down
            remove_column :records, :kentucky_topic_id, :integer 
         end

The syntax may not all be correct but I will verify it before running rake db:migrate

[rrmait0]

Also, remove the old index from records:

remove_index: records, kentucky_topic_id

[rrmait0]

I'm getting a SQLite3 error

SQLite3::SQLException: SQL logic error or missing database: INSERT INTO "kentucky_topics_records" ("record_id", "kentucky_topic_id") VALUES (1,2)

[cokernel]

I'll try to reproduce the error tomorrow morning.

[cokernel]

I cloned a fresh copy, installed database.yml, checked out master, and ran rake db:setup.

Then I pulled the kentucky_topics branch and ran rake db:migrate. The final migration failed:

==  DropKentuckyTopicIndexFromRecords: migrating ==============================
-- remove_index(:records, :kentucky_topic_id)
rake aborted!
An error has occurred, this and all later migrations canceled:

Index name 'index_records_on_kentucky_topic_id' on table 'records' does not exist

The previous migration (Drop kentucky topics from records) dropped the column the index was referencing, causing the index itself to be dropped as well. This is why the migration fails.

I'll follow up with recommendations.

[cokernel]

Fixing this will be easier if you first revert to master and delete the unnecessary migration:

$ git checkout master
$ rake db:reset
$ git checkout kentucky_topics
$ git rm db/migrate/20130826202941_drop_kentucky_topic_index_from_records.rb

However, there is still a problem with db/migrate/20130826202537_drop_kentucky_topics_from_records.rb. Specifically, the remove_column method is not reversible and is therefore invalid for inclusion in a change method in a migration. The complete list of reversible methods is available in the Active Record Migrations guide in Section 3.5.

To create a reversible migration that drops a column, you need the up method that drops the column and a down method that recreates the column (with appropriate type) and, if necessary, the index. Look at db/migrate/20130823135616_drop_file_format.rb for an example of a reversible migration that (among other things) drops a column.

Once you modify db/migrate/20130826202537_drop_kentucky_topics_from_records.rb to use up and down methods instead of change, it would be good to verify that rake db:migrate runs without any errors before committing and pushing the new code.

[rrmait0]

Okay, I understand what I need to do. How about the previous migration: db/migrate/20130826191422_create_kentucky_topics_records_table.rb Do I need to modify this migration as well?

[cokernel]

Both create_table and add_index are reversible, so I would drop the down method and rename the up method to change. I recommend fixing the indentation for the method as well; lines 10-12 should be aligned with create_table and not with the create_table block.

[rrmait0]

I updated the migrations according to the recommendations.

db/migrate/20130826191422_create_kentucky_topics_records_table.rb

class CreateKentuckyTopicsRecordsTable < ActiveRecord::Migration
  def change                   
    create_table :kentucky_topics_records, :id=> false do |t| 
     t.string :name 

     t.timestamps              

     t.references :kentucky_topic    
     t.references :record      
    end 
    add_index :kentucky_topics_records, [:kentucky_topic_id, :record_id]
    add_index :kentucky_topics_records, :record_id
  end 
end 

db/migrate/20130826202537_drop_kentucky_topics_from_records.rb

class DropKentuckyTopicsFromRecords < ActiveRecord::Migration
  def up
    remove_column :records, :kentucky_topic_id
  end

  def down
    add_column :records, :kentucky_topic_id, :integer
    add_index  :records, :kentucky_topic_id
  end
end

This is what I received after running the migrations above:

==  CreateKentuckyTopicsRecordsTable: migrating ===============================
-- create_table(:kentucky_topics_records, {:id=>false})
   -> 0.0083s
-- add_index(:kentucky_topics_records, [:kentucky_topic_id, :record_id])
   -> 0.0005s
-- add_index(:kentucky_topics_records, :record_id)
   -> 0.0005s
==  CreateKentuckyTopicsRecordsTable: migrated (0.0094s) ======================

==  DropKentuckyTopicsFromRecords: migrating ==================================
-- remove_column(:records, :kentucky_topic_id)
   -> 0.0786s
==  DropKentuckyTopicsFromRecords: migrated (0.0787s) =========================

It looks like it ran okay. However, I'm still receiving the same error message:

ActiveRecord::StatementInvalid in RecordsController#update

SQLite3::SQLException: SQL logic error or missing database: INSERT INTO "kentucky_topics_records" ("record_id", "kentucky_topic_id") VALUES (1, 1)

Rails.root: /opt/pdp/services/apps/users/rrmait0/metadata_editor

[cokernel]

What steps are required to produce this error?

[rrmait0]

When creating a new or editing a existing record:

In the "Kentucky topics" field add some topics and click on "Create Record"

The above error is generated. If the "Kentucky topics" field is blank there is no error.

[rrmait0]

I believe the problem is with the kentucky_topics_records table. It should not have the filed name and created/updated_at fields. Similar to the language_records table with only has language_id and record_id fields. Therefore, the kentucky_topics_records table only should have record_id and kentucky_topic_id fields. Therefore, a migration needs to be generated to remove those fields or just drop that table and re-create the table with the new fields. Take a look at schema.rb and let me know if the steps I'm proposing are correct.

[cokernel]

I think you are right about the extra fields being the problem. I tried performing the insert directly in SQLite and got a more useful error message:

$ sqlite3 db/development.sqlite3
SQLite version 3.3.6
Enter ".help" for instructions
sqlite> INSERT INTO "kentucky_topics_records" ("record_id", "kentucky_topic_id") VALUES (1, 1);
SQL error: kentucky_topics_records.created_at may not be NULL
sqlite>

Since we haven't merged this into master yet, I'm going to recommend that you edit the migration that creates the kentucky_records_topics table instead of generating a new migration. (You'll probably have to run rake db:reset to get something sane afterwards.) You can drop the t.string :name and t.timestamps lines from the original migration.

[rrmait0]

I updated the code and it works fine. It's ready to be merged with master.

[cokernel]

Looks good. Merging.