-
Notifications
You must be signed in to change notification settings - Fork 7
Converting legacy system cards to custom cards
-
On your local system, use the admin card editor to build the correct XML for the card you are creating
-
Close all existing database connections to your local server
-
Load production data:
- rake db:import_remote
-
Create a new config file and database migration for the custom card:
- rails generate custom_card:configuration "Title of New Card" "TahiStandardTasks::ClassNameOfOldTask"
-
Edit the newly generated configuration file (found in lib/custom_card/configurations)
- Update the xml_content method with the XML for the new card
- Ensure that all the other settings are the desired behavior
-
Perform the data migration (that was generated by the rails generator earlier):
- rake db:migrate
-
Remove any existing code referring to the old task
- be sure to look for all camelized and dasherized versions of the task name
-
As your last commit, be sure to dump local data that contains your conversion
- bin/update_data
- A configuration file containing the XML of the new custom card
- A migration naming the title of the new card, and the old class name (TahiStandardTasks::<name>)
Both of these are created with the "rails generate" command from above. Here is an example of creating migration artifacts for the Supporting Information task:
tahi $ rails generate custom_card:configuration "Supporting Information" "TahiStandardTasks::SupportingInformationTask"
** [Bugsnag] Bugsnag exception handler 5.0.1 ready
You do not curently have production data loaded. Existing permissions will not be derived. Want to continue? y
create lib/custom_card/configurations/supporting_information.rb
generate custom_card:migration
** [Bugsnag] Bugsnag exception handler 5.0.1 ready
create db/migrate/20170829141700_migrate_supporting_information_task_to_custom_card.rbTwo files were created the first, db/migrate/20170829141700_migrate_supporting_information_task_to_custom_card.rb, looks like this:
class MigrateSupportingInformationTaskToCustomCard < ActiveRecord::Migration
def up
# load custom card into the system
CustomCard::Loader.all
# migrate legacy task to custom card
migrator = CustomCard::Migrator.new(legacy_task_klass_name: "TahiStandardTasks::SupportingInformationTask", card_name: "Supporting Information")
migrator.migrate
end
endNote that it explicitly invokes the card loader, which will add the card defined by the XML in the second file, before using the CustomCard::Migrator class to migrating legacy answers and tasks to point to the new card.
The second file, lib/custom_card/configurations/supporting_information.rb, looks like this:
# rubocop:disable Metrics/MethodLength
module CustomCard
module Configurations
#
# This class defines the specific attributes of a particular
# Card and it can be used to create a new valid Card into the
# system via the CustomCard::Loader.
#
class SupportingInformation < Base
def self.name
"Supporting Information"
end
def self.view_role_names
["Academic Editor", "Billing Staff", "Collaborator", "Cover Editor", "Creator", "Handling Editor", "Internal Editor", "Production Staff", "Publishing Services", "Reviewer", "Staff Admin"]
end
def self.edit_role_names
["Collaborator", "Cover Editor", "Creator", "Handling Editor", "Internal Editor", "Production Staff", "Publishing Services", "Staff Admin"]
end
def self.view_discussion_footer_role_names
view_role_names
end
def self.edit_discussion_footer_role_names
edit_role_names
end
def self.publish
true
end
def self.do_not_create_in_production_environment
false
end
def self.xml_content
<<-XML.strip_heredoc
<?xml version="1.0" encoding="UTF-8"?>
<card required-for-submission="true" workflow-display-only="false">
<content content-type="display-children">
</content>
</card>
XML
end
end
end
end
# rubocop:enable Metrics/MethodLengthThe XML heredoc then needs to be edited to define the layout of the new card, matching each ident that the legacy card used. These can be enumerated by following along with my rails console session below.
- Legacy cards aren't associated with journals, custom cards are.
- Replacement card XML must have idents that match legacy card.
- Existing TaskTemplates must be updated to have a nil journal task type and point to the new card.
- Existing Answers for each ident must be updated to point to new CardContents.
- Existing Tasks must be updated to have the type "CustomCardTask", and point to the replace card's current published version.
- Tasks without a backing model can't be invoked as ActiveRecord objects, so updates need to happen in SQL-space, without the Task model getting instantiated.
- There are many places in the Tahi codebase that assume cards can be differentiated by class name. This required some refactoring in 10097, and this will continute to come up as we migrate more cards.
- A handful of places post-10097 now check for a Task type being "CustomCardTask", and branch to alternate logic, notably the Task model's `submission_task?` method and the metadata_serializer's call to custom_task.
- The TaskFactory spec needs to alter factory definitions to instantiate CustomCardTasks if they refer to a card being converted.
- CardVersionController permissions were tricky since card versions aren't explicitly associated with papers. We switched to embedding version objects in the CustomCardTaskSerializer, to prevent the controller from being invoked.
- Other migrations instantiating Tasks or altering the CardContent schema can conflict with conversions. An example of this appears in the "Failed Migrations" section below.
This section has a target audience of developers, but if you're relatively rails or database fluent, please follow along!
To begin with, let's examine how Competing Interests data looked prior to it's migration to a custom card. The last commit prior to that was 6a4558cc0, which I have checked out prior to opening a rails console.
tahi $ git status
HEAD detached at 6a4558cc0
nothing to commit, working tree clean
tahi $ rails c
** [Bugsnag] Bugsnag exception handler 5.0.1 ready
Loading development environment (Rails 4.2.7.1)
System tasks were previously referenced by their class name. Tasks for custom cards have a single class name of CustomCardTask, so a lot of refactoring was necessary to not derive behavior from class name, but to instead check things like card titles and idents. This is still a work in progress. Here we're starting with cards whose name has the old CompetingInterestsTask class.
[1] pry(main)> card = Card.find_by(name: 'TahiStandardTasks::CompetingInterestsTask')
=> #<Card:0x007fa0a11612c0
id: 42,
created_at: Tue, 14 Mar 2017 21:17:08 UTC +00:00,
updated_at: Wed, 03 May 2017 15:50:01 UTC +00:00,
deleted_at: nil,
name: "TahiStandardTasks::CompetingInterestsTask",
journal_id: nil,
latest_version: 1,
archived_at: nil,
state: "locked">
I used "find_by" instead of "where" because there was only one card. Note that it's journal ID was nil, meaning it was a system card not associated with any particular journal. Custom Cards are journal-specific, so when cards are converted, they need to be added to each journal, which is now done by the rake task cards:load. More on that later.
Cards have versions. System cards were retroactively assigned a version at some point during the introduction of custom cards, but since custom cards can be modified, we needed a way to associate answers with the version of the card that asked the question. The "latest_published_card_version" convenience method finds the current version, where system cards always have version 1.
[2] pry(main)> version = card.latest_published_card_version
=> #<CardVersion:0x007fa09ee71328
id: 17,
version: 1,
card_id: 42,
deleted_at: nil,
required_for_submission: false,
published_at: Wed, 03 May 2017 15:50:01 UTC +00:00,
published_by_id: nil,
history_entry: nil,
workflow_display_only: false>
Card versions have card contents, which represent the questions to be presented to whoever is editing the task. The "ident" field of a CardContent record should be unique (not used in other cards, but they should be used in other versions of the same card), and they are used later in the paper stream. Answers corresponding to the ident below, "competing_interests–statement", are queried for by the typesetter export process, for example.
[3] pry(main)> contents = version.card_contents; contents.count
=> 3
[4] pry(main)> contents.last
=> #<CardContent:0x007fa0a0496040
id: 134,
ident: "competing_interests--statement",
parent_id: 133,
lft: 265,
rgt: 266,
text:
"Please provide details about any and all competing interests in the box below. Your response should begin with this statement: \"I have read the journal's policy and the authors of this manuscript have the following competing interests.\"",
value_type: "html",
created_at: Tue, 14 Mar 2017 21:17:08 UTC +00:00,
updated_at: Tue, 20 Jun 2017 23:35:13 UTC +00:00,
deleted_at: nil,
card_version_id: 17,
content_type: nil,
placeholder: nil,
possible_values: nil,
visible_with_parent_answer: nil,
label: nil,
default_answer_value: nil,
allow_multiple_uploads: false,
allow_file_captions: false,
editor_style: nil,
allow_annotations: false,
instruction_text: nil>
When converting a system card to a Custom Card, you will need to locate all the idents, so that they can be included as attributes in the Custom Card XML. I'll show you an example of that further down. Once you have grabbed the card contents records, pull out their ident fields and squash nils like so:
[5] pry(main)> contents.pluck(:ident).compact
=> ["competing_interests--has_competing_interests", "competing_interests–statement"]
When a paper is submitted, tasks get snapshotted with a hash containing card contents and their corresponding answers. This is used in the versions view diff. Here is an example of grabbing a legacy Competing Interests task snapshot, and pulling out the answer history for the competing interests statement.
[6] pry(main)> tasks = TahiStandardTasks::CompetingInterestsTask.all; tasks.count
=> 9
[7] pry(main)> Snapshot.where(source: tasks).first
=> #<Snapshot:0x007fa0a01c7fa8
id: 37,
source_type: "Task",
source_id: 174,
paper_id: 12,
major_version: 0,
minor_version: 0,
contents:
{"name"=>"competing-interests-task",
"type"=>"properties",
"children"=>
[{"name"=>"competing_interests--has_competing_interests",
"type"=>"question",
"value"=>
{"id"=>38,
"title"=>
"Do any authors of this manuscript have competing interests (as described in the <a target='_blank' href='PLOS'>http://journals.plos.org/plosbiology/s/competing-interests'>PLOS Policy on Declaration and Evaluation of Competing Interests</a>)?",
"answer_type"=>"boolean",
"answer"=>false,
"attachments"=>[]},
"children"=>
[{"name"=>"competing_interests--statement",
"type"=>"question",
"value"=>
{"id"=>39,
"title"=>
"Please provide details about any and all competing interests in the box below. Your response should begin with this statement: \"I have read the journal's policy and the authors of this manuscript have the following competing interests.\"",
"answer_type"=>"text",
"answer"=>"The authors have declared that no competing interests exist.",
"attachments"=>[]},
"children"=>[]}]},
{"name"=>"id", "type"=>"integer", "value"=>174}]},
created_at: Tue, 12 Apr 2016 18:55:57 UTC +00:00,
updated_at: Tue, 12 Apr 2016 18:55:57 UTC +00:00,
key: nil>
[8] pry(main)> Snapshot.where(source: Task.find(174)).map { |s| s.contents['children'][0]['children'][0]['value']['answer'] }
=> ["The authors have declared that no competing interests exist.",
"The authors have declared that no competing interests exist.",
"I forgot I have a competing interest. I'm reviewing a paper on this same topic.",
"I forgot I have a competing interest. I'm reviewing a paper on this same topic.",
"I forgot I have a competing interest. I'm reviewing a paper on this same topic."]
[9] pry(main)> quit
When a card gets migrated, the XMLCardLoader first tries to load the new card from the XML source. If the same release contains a change to the CardContent schema, this can cause a failure during the migration, that may look something like this:
tahi $ git checkout master
Previous HEAD position was 6a4558cc0... Simplify implementation
Switched to branch 'master'
Your branch is up-to-date with 'origin/master'.
tahi $ rake db:migrate
** [Bugsnag] Bugsnag exception handler 5.0.1 ready
Migrating to AddTargetIdentAndViolationValueToCardContentValidations (20170630165801)
== 20170630165801 AddTargetIdentAndViolationValueToCardContentValidations: migrating
-- add_column(:card_content_validations, :target_ident, :string)
-> 0.0009s
-- add_column(:card_content_validations, :violation_value, :string)
-> 0.0005s
-- add_index(:card_content_validations, :target_ident)
-> 0.0037s
-- add_index(:card_content_validations, :violation_value)
-> 0.0031s
== 20170630165801 AddTargetIdentAndViolationValueToCardContentValidations: migrated (0.0085s)
Migrating to AddRevertChildrenOnHideToCardContent (20170711225015)
== 20170711225015 AddRevertChildrenOnHideToCardContent: migrating =============
-- add_column(:card_contents, :revert_children_on_hide, :boolean)
-> 0.0006s
== 20170711225015 AddRevertChildrenOnHideToCardContent: migrated (0.0007s) ====
Migrating to AddRequiredFieldToCardContent (20170713222658)
== 20170713222658 AddRequiredFieldToCardContent: migrating ====================
-- add_column(:card_contents, :required_field, :boolean, {:default=>false})
-> 0.0225s
== 20170713222658 AddRequiredFieldToCardContent: migrated (0.0226s) ===========
Migrating to CustomCardCompetingInterests (20170721145227)
== 20170721145227 CustomCardCompetingInterests: migrating =====================
Running rake task: custom_card:convert_legacy_task
Loading legacy Cards unattached to any specific Journal ...
Loading Custom Cards attached to each Journal ...
rake aborted!
StandardError: An error has occurred, this and all later migrations canceled:
unknown attribute 'condition' for CardContent.
/Users/curtis/dev/plos/tahi/app/services/xml_card_loader.rb:70:in `build_card_content'
/Users/curtis/dev/plos/tahi/app/services/xml_card_loader.rb:57:in `block in build_card_contents'
...truncated...
Tasks: TOP => db:migrate
(See full trace by running task with --trace)
In this case, a schema change to add a new column to card_contents was referenced in app/services/xml_card_loader.rb, but had not yet been added to the database. The card migration tried to instantiate a record with the new schema before the database was ready for it. In this particular case, the content attributes. The safest way to prevent this is to make sure card migrations happen last. This is worth calling out mainly because of all the current (Aug 2017) changes to the Custom Card schema as we migrate more cards.
Another possible migration gotcha is when other migrations may instantiate tasks whose backing class no longer exists, prior to their types being updated in the database. For example, in the 1.47 release, prior to the data migrations running, tasks with type TahiStandardTasks::CompetingInterestsTask still existed in the database, and were queued for having their types and card versions updated. However, if another migration tried to iterate over tasks and instantiate their ActiveRecord models, something like this would happen:
tahi $ git checkout release/1.47
Switched to branch 'release/1.47'
Your branch is up-to-date with 'origin/release/1.47'.
tahi $ rails c
** [Bugsnag] Bugsnag exception handler 5.0.1 ready
Loading development environment (Rails 4.2.7.1)
[1] pry(main)> Task.where(type: 'TahiStandardTasks::CompetingInterestsTask').first
ActiveRecord::SubclassNotFound: The single-table inheritance mechanism failed to locate the subclass: 'TahiStandardTasks::CompetingInterestsTask'. This error is raised because the column 'type' is reserved for storing the class in case of inheritance. Please rename this column if you didn't intend it to be used for storing the inheritance class or overwrite Task.inheritance_column to use another column for that information.
from /Users/curtis/.rbenv/versions/2.2.6/lib/ruby/gems/2.2.0/gems/activerecord-4.2.7.1/lib/active_record/inheritance.rb:186:in `rescue in find_sti_class'
[2] pry(main)> quit
tahi $
Because of both of these possible migration problems, great care should be taken prior to a release where you are migrating a card. Prior to the release being cut, check out master, pull in a copy of the production database, and run db:migrate. Your migration may run fine locally in your own branch, but it's important to see if it behaves with the other migrations pending in the same release.