Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tweak the Item / CanonicalItem association to use the partner_key trait instead of the integer key #588

Closed
armahillo opened this issue Oct 28, 2018 · 0 comments

Comments

@armahillo
Copy link
Collaborator

Currently

class CanonicalItem < ApplicationRecord
  has_many :items, dependent: :destroy
  validates :partner_key, presence: true, uniqueness: true
class Item < ApplicationRecord
  belongs_to :canonical_item, counter_cache: :item_count

Item has a canonical_item_id field that's used as the foreign key in its association. CanonicalItem effectively has 2 keys -- id and partner_key. The problem is that when the app has places where Item and :partner_key intersect, we end up having to look up the CanonicalItem table first, and then do the Item lookups. (basically: converting one key to the other)

Desired change

We can keep id for now since it's the Rails Way(tm) and it's not hurting anything (ie. any actions where CanonicalItem is the resource can remain as-is) -- but the association between Item and CanonicalItem should be based on the partner_key field instead of id / canonical_item_id. (ie. any actions where Item is the resource and it needs to reference its canonical_item association may need to be changed)

Something like this should work

belongs_to :canonical_item, counter_cache: :item_count, foreign_key: :partner_key

with an accompanying migration adjustment to the Items table to add a partner_key:string column, and migrate the previous canonical_item_id association to it (N.B. you'll need to pull CanonicalItem.all first -- there's < 50 records so just load it into memory).

Add a scope called by_partner_key. (should should facilitate solving #394)

Changes will also need to be made in controllers, seed files, factories, and basically any file that comes up with grep -lR canonical_item_id. The test suite should reveal any missing changes. Be sure to include model spec coverage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant