Replace Card Resource & API with a Unified Card backing view. #107
Conversation
plural
changed the title
|
|
||
| def latest_printing_id | ||
| @model.printings.max_by { |p| p.date_release } ['id'] | ||
| @model.printing_ids[0] |
There was a problem hiding this comment.
Is this now guaranteed to be enough?
There was a problem hiding this comment.
This caused me to look at the view definition again. I was sorting by printing id descending, but i changed to use date_release DESC instead. Results are the same, but it communicates the intent more clearly.
Since the view has the field sorted, taking the first will be correct.
| 'gains_subroutines' => 'unified_cards.gains_subroutines', | ||
| 'h' => 'unified_cards.trash_cost', | ||
| 'has_global_penalty' => 'unified_cards.restrictions_global_penalty', | ||
| 'in_restriction' => 'unified_cards.in_restriction', |
There was a problem hiding this comment.
in_restriction appears to be either true or null, making it (I think) impossible to search for cards not in restrictions
There was a problem hiding this comment.
ooh, good catch! Fixed.
|
Ok, i think this is in good shape for a review. I have a couple TODOs but we can deal with that later. |
| 'side' => 'unified_cards.side_id', | ||
| 'snapshot' => 'unified_cards.snapshot_ids', | ||
| 'strength' => 'unified_cards.strength', | ||
| 't' => 'unified_cards.card_type_id', |
There was a problem hiding this comment.
type:... and subtype:... are missing from the accepted syntax
There was a problem hiding this comment.
we have card_type and card_subtype already, actually. I'd prefer to keep other syntax changes not related to the backing view change out of this PR, but let's revisit when i put the syntax doc page together. sg?
|
Thanks for taking a look! I went through and verified all the relationships to cards are updated and work. |
|
oh, i have something still to do for card_pool and i'll fix the other 2 syntax issues you found. |
|
actually, card_pool's issue was a local diff on my side, but the PR is good. |
2 participants
I am pretty happy with the view, but will wire up more pieces in the model and in the card API & query builder before declaring victory.