Y24-097 Asset renaming - Barcode model - code references
KatyTaylor opened this issue · 2 comments
Describe the Housekeeping
A Barcode has a Labware, as shown in the following code in the Barcode model:
belongs_to :asset, optional: false, class_name: 'Labware'
The name of this relationship is asset
despite the table and model being called Labware
.
This story is to add an alias to allow use of the more appropriate relationship name, and update all code references to use the alias.
A future story will be made to rename the database column.
Tasks
- Add aliases to the Barcode model to allow use of the relationship name
labware
. Suggested aliases as follows:
alias_attribute :labware_id, :asset_id
alias_attribute :labware, :asset
- Find all code references to this relationship and alter them to use the above aliases.
- A possible test to see if they have all been found is to rename the
asset_id
column onbarcodes
locally and remove the alias, and run tests to see what errors. - Write up any learnings as you go, to help with future stories on different models.
Additional context
This story came out of the following research story. This has more detail and results of investigations that could be useful to read:
Hi @KatyTaylor, I've added an alias with alias_association
, which adds a getter and a setter, similar to what happens when we use alias_attribute
. We could have used alias_attribute
, but I came to know that it was designed for aliasing attributes. So, with the changes I've made, it should be possible to address asset
and labware
relations synonymously in Rails web entities (models, controllers) when it comes to the semantics.
Regarding the third task listed in the description: I've tried doing this, but I think it is not possible to come to a working state by just renaming the column and removing the alias. In terms of alias_attribute
, it's giving the programmer flexibility to refer to a certain table column with a different name. For instance, it gives the ability for us to refer the column asset_id
as labware_id
within our business logic; so for this, the table should have asset_id
column, and labware_id
is just a pointer to it. Furthermore, I think that it will violate the validations if we just rename the column (I've tried doing this, and validations do fail), as we then would be trying to rename columns that are part of foreign key constraints. I think the way to do this is through a migration. I think it might be something like:
class RenameOldColumnNameToNewColumnName < ActiveRecord::Migration[6.1]
def change
# Remove the foreign key constraint
remove_foreign_key :barcodes, :labware
# Rename the column
rename_column :barcodes, :asset_id, :labware_id
# Re-add the foreign key constraint
add_foreign_key :barcodes, :labware, column: :labware_id
end
end
It is my opinion that this migration needs to be part of #4123, as it involves the schema changes. I do not think that there will be a lot of changes required further (in case I haven't missed stuff) in our business logic in terms of the association name, but I do suspect that we would likely to get prompts in test factories for some changes after we add the migration.
Good find, I hadn't seen that article about misuse of alias_attribute
. If you've found another way that makes it "possible to address asset and labware relations synonymously in Rails web entities", then that's great, that's what I was after.
Renaming the field - when I wrote that acceptance criteria, I was trying to think of ways we could be sure we'd found all the code references to the 'old' name. It sounds like it's not as quick a task as I thought to make the schema change locally, temporarily, to smoke out hidden references. If you think it makes more sense to wait until the next story, and accept we may have some further code changes as part of that, then that's fine - it doesn't really matter which story it's done as part of.
Thanks for looking into this so thoroughly, looks like you're making good discoveries that will help with the next model too.