4175 add request type flag #4271
Conversation
…s export. Adds filter by request type to index.
…s export. Adds filter by request type to index.
| @@ -19,6 +20,7 @@ def call | |||
| return self unless valid? | |||
|
|
|||
| request_create_svc = Partners::RequestCreateService.new( | |||
| request_type: @request_type.to_i, | |||
There was a problem hiding this comment.
This service should already know the request type just by whether for_families is true or false.
There was a problem hiding this comment.
I feel that defining request_type in this way is more transparent. Would you prefer to add logic in the controller to determine request_type rather than using the instance var?
There was a problem hiding this comment.
I'd prefer putting the business logic as far back as possible. The controller is calling a service, which already has to know what kind of request it's creating. We can rely on the service to set the request type.
|
Understood. The rest still needs @dorner 's technical ok -- it's not crucial for it to get in this week. |
…in view to display only the first letter, pull request_type from service instead of defining with an instance var.
…75-add-request-type-flag
…man-essentials into 4175-add-request-type-flag
| @@ -13,7 +13,7 @@ | |||
| <%= request_row.comments %> | |||
| </td> | |||
| <td> | |||
| <%= request_row&.request_type&.titleize %> | |||
| <%= request_row&.request_type&.first&.capitalize %> | |||
There was a problem hiding this comment.
Can we extract this into a method on request? It's a bit obscure. If you call it something like request_type_label with a comment it'd be more obvious.
db/schema.rb
Outdated
| @@ -849,7 +849,8 @@ | |||
| end | |||
|
|
|||
| create_table "versions", force: :cascade do |t| | |||
| t.string "item_type", null: false | |||
| t.string "item_type" | |||
| t.string "{:null=>false}" | |||
There was a problem hiding this comment.
I don't think this is supposed to be here?
There was a problem hiding this comment.
This was automatically changed when I ran the migrations last. I reverted it.
There was a problem hiding this comment.
Tried it on my local. Signed in as verified@example.com, entered a quantity request and got
|
Same effect for individual and child request. (Not at all surprised by that) |
|
Well that is embarrassing. I'll take a look at it this afternoon. |
|
Seems I either forgot to add or removed request_type from the initializers. I have replaced them and tested locally. Everything should work as expected now. |
…man-essentials into 4175-add-request-type-flag
|
There's an issue with setup that's currently blocking me doing functional reviews -- just managing your expectations as to turnaround time. |
| @@ -8,7 +8,7 @@ class FamilyRequestCreateService | |||
|
|
|||
| attr_reader :partner_user_id, :comments, :family_requests_attributes, :partner_request | |||
|
|
|||
| def initialize(partner_user_id:, family_requests_attributes:, comments: nil, for_families: false) | |||
| def initialize(request_type:, partner_user_id:, family_requests_attributes:, comments: nil, for_families: false) | |||
There was a problem hiding this comment.
Wait, why was this put back? We agreed that it wouldn't be needed because the service itself knows what type it is just by the current parameters being passed in.
|
Had a question... also, lint is failing now. |
Resolves #4175
Description
Adds a request type flag to index and show pages as well as to the downloadable document so that banks can better see which type of request is being made.
Type of change
This can be manually tested by logging in as a bank user.
Screenshots
Requests-2024-04-02.csv