Add service to order copy cards #570
Conversation
From: https://eaflood.atlassian.net/browse/RUBY-767 Add a reusable service that given a registration, a user, a number of copy cards and a user, generates a copy cards only order
There was a problem hiding this comment.
I started going through what WCR Frontend does and found
- https://github.com/DEFRA/waste-carriers-frontend/blob/master/app/controllers/order_controller.rb
- https://github.com/DEFRA/waste-carriers-frontend/blob/568296e712e16cb00422cdb09e0a49469ff1daa9/app/models/registration.rb#L1620
- https://github.com/DEFRA/waste-carriers-frontend/blob/568296e712e16cb00422cdb09e0a49469ff1daa9/lib/registration_order.rb
- https://github.com/DEFRA/waste-carriers-frontend/blob/568296e712e16cb00422cdb09e0a49469ff1daa9/lib/order_builder.rb
So gave up!
From what I'm reading it looks correct. I'm just commenting on what order to do things
- add the new functionality and then refactor the current order method
- or refactor the existing process first, then have the new functionality follow the same pattern
I'm voting for the later but could be shouted down.
| @@ -25,19 +25,12 @@ class Order | |||
| field :manualOrder, as: :manual_order, type: String | |||
| field :order_item_reference, type: String | |||
|
|
|||
| # TODO: Move to a service | |||
There was a problem hiding this comment.
If the intention is to move this to an OrderRenewalService (or something like that) would it be better to do that refactoring first and then add your new service?
As is I'm just not sure I like the idea that if we never get round to this TODO we'll have Order create the order with new_order, set the order items as if its a renewal, but then OrderAdditionalCardsService take that order and simply overwrites the array. Plus we're creating orders in 2 different ways.
There was a problem hiding this comment.
This service do not use the code in new_order so I have left the TODO for the next person touching it. We should get there naturally when we work out the front end flow.
There was a problem hiding this comment.
🤦♂ Yep, was pointed out to me I was misreading the changes!
cintamani
changed the title
| @@ -25,19 +25,12 @@ class Order | |||
| field :manualOrder, as: :manual_order, type: String | |||
| field :order_item_reference, type: String | |||
|
|
|||
| # TODO: Move to a service | |||
There was a problem hiding this comment.
🤦♂ Yep, was pointed out to me I was misreading the changes!
cintamani
changed the title
|
I am pretty sure I actually need this as it is now, so I am merging this in |
From: https://eaflood.atlassian.net/browse/RUBY-767
Add a reusable service that given a registration, a user, a number of copy cards and a user, generates a copy cards only order