836 refactor reservations controller #882
Conversation
|
Caught a small typo. I also think that it's somehow not putting a newline before the checkin / checkout notes headings, not sure why that is:
|
| # new_notes (from the form), procedure_kind (:checkin or :checkout) and array | ||
| # of string steps of procedures that were not followed for procedure_kind. | ||
| def make_notes(old_notes, new_notes, procedure_kind, procedures) | ||
| notes = old_notes ? old_notes + "\n\n" : '' |
There was a problem hiding this comment.
got it, it was this line. We need to add this back in down below.
|
Fixed the whitespace issue just by prefixing the note header with a \n, and also converted the heading to markdown syntax |
|
Also, improved test coverage to actually test for the situations and urls where the checkout/checkin actions should trigger a redirect |
|
It looks like every time you edit a reservation then it adds a header to the notes, even if no notes were entered. We should do one of two things:
What do you think? |
|
Hard to say, at this point it seems notes has really grown into a powerful recordkeeper. Adjusting the logic to add notes only when new notes were added was trivial, but I do like the idea of also logging edit changes. |
|
I think we should log edit changes; [nominally] immutable records of history seems really really useful. I think it falls under this issue, once we get that implemented and tested we can pull the whole thing in. |
|
Ok, this now logs all edited parameters to the notes, along with any notes that were added. Let me know if this looks ok, then I think we're good to go! |
|
Fixed some tests failing due to old |
|
Line 141 was taken out. Is that notes variable necessary? |
|
@coollog in which file? In general, you can add a comment to a file by clicking the blue plus next to the relevant line in the "Files Changed" view (hover over the line to see the button). |
| @@ -136,12 +130,8 @@ def update # for editing reservations; not for checkout or check-in | |||
| end | |||
|
|
|||
| # save changes to database | |||
| @reservation.update_attributes(res) | |||
| if params[:new_notes] | |||
| @reservation.notes = @reservation.notes.to_s + "\n#### New notes added at #{Time.current.to_s(:long)} by #{current_user.name}\n" + params[:new_notes] | |||
There was a problem hiding this comment.
Sorry for the late response, but yep! That's the file.
There was a problem hiding this comment.
Yea I figured it out afterwards and meant to update my comment. Do you mind
clarifying what you were asking?
There was a problem hiding this comment.
I was just wondering since this was completely taken out, if this notes was needed anywhere else.
There was a problem hiding this comment.
Got it. That functionality should be essentially replicated in the
replacement model method (i.e. we make note of when someone added notes to
a reservation with a header of some kind). I believe this falls under the
update method). Does that make sense?
Conflicts: app/views/admin_mailer/_reservation_note.html.erb db/schema.rb lib/tasks/mailman.rake
…ller resolves #836 and a whole bunch of others
Resolves #836
Actually, I'm going to go and write tests for this so don't merge until then