1058 Add Feedback Subject #1116
Conversation
pr changes
| @feedback = Feedback.new(feedback_params) | ||
|
|
||
| if @feedback.valid? | ||
| FeedbackMailer.with(feedback_params).send_message.deliver_later |
There was a problem hiding this comment.
Let's use deliver_now for the time being, until we set up background processing.
|
How does this look? |
| end | ||
| end | ||
|
|
||
| def path |
There was a problem hiding this comment.
I have been starting at this method as it just doesn't quite feel 'right'. We are defining the path based on the layout. It works. But the layout is defined based on whether there is no current_user, the user is an adopter, the user is a staff. It seems to me we should use the same checks for the redirect path. Using the layout feels like it's a proxy, and it's coupled with the layout unnecessarily. The driver here should be the user type, or lack thereof. Thoughts on this?
It's not a hill I am going to die on...but I think it would be clearer.
There was a problem hiding this comment.
For sure. I think I originally was trying to keep things DRY and use the set_layout method and I kinda just went down that hole. Then I was lazy and just moved the case into a method without thinking about it more.
Maybe something like
def path
if current_user.nil?
root_path
elsif current_user.has_role?(:adopter, ActsAsTenant.current_tenant)
adopter_fosterer_dashboard_index_path
else
staff_dashboard_index_path
end
end
Same as set_layout but returning the path instead. Maybe there is a simple way to use set_layout for both but I can't think of it at the moment or even if that is a smart idea.
There was a problem hiding this comment.
Yes I think that works ^. I also wondered about setting both at once...but single responsibility is probably better to stick to when in doubt!
There was a problem hiding this comment.
looks great @jmilljr24! Just one comment to consider on that private method. Your call on what you think is best.
#1058
✍️ Description
A required dropdown selection is added for Subject on the feedback form. Additionally, a link is added to updated a turbo_frame with additional information on how to submit feedback. The placement of the link and/or displayed information could use improvement.
📷 Screenshots/Demos