-
Notifications
You must be signed in to change notification settings - Fork 15
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
5646 item direction table #5752
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Chris, seems to be working well for me!
Couple of comments up to you if you want to address them...
|
||
#[derive(Clone, Default, PartialEq, Debug)] | ||
pub struct ItemDirectionFilter { | ||
pub id: Option<EqualFilter<String>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're going to want a filter on item_id
, could be done later though, so I'm happy if it's not in this PR.
id TEXT NOT NULL PRIMARY KEY, | ||
item_link_id TEXT NOT NULL REFERENCES item_link(id), | ||
directions TEXT NOT NULL, | ||
priority BIGINT NOT NULL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have thought INTEGER
would be ok, I guess big int is fine too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had integer and diesel postgres was throwing a wobbly on i64 not receiving enough bytes
|
||
let result = ItemDirectionRow { | ||
id: data.ID, | ||
item_link_id: data.item_ID, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it safe to assume an item_id
is also an item_link_id
? Feels like we might need to look up the item_id
based on this, but then I always get confused with *_link
tables...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The item link tables are populated with item.id == item_link.id == item.item_link_id
so it should be fine I think!
Clone, Insertable, Queryable, Debug, PartialEq, AsChangeset, Eq, Serialize, Deserialize, | ||
)] | ||
#[diesel(table_name = item_direction)] | ||
#[diesel(treat_none_as_null = true)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is a little bit pointless with no nullable columns :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cargo cult π
Fixes #5646
π©π»βπ» What does this PR do?
Adds migration, repo and sync logic for the
item direction
table