Bug 29007

Created attachment 158337 [details] [review]
Bug 29007: Add cancellation reason input on check in

Test plan:

1) Check an item in with already a confirmed hold
2) Cancel the hold, it was cancelled without possibility to choosing a
   reason
3) Apply this patch and do it again, you must see an input to choose
   your cancellation reason (list from VA)

Created attachment 158473 [details] [review]
Bug 29007: Add cancellation reason input on check in

Test plan:

1) Check an item in with already a confirmed hold
2) Cancel the hold, it was cancelled without possibility to choosing a
   reason
3) Apply this patch and do it again, you must see an input to choose
   your cancellation reason (list from VA)

Signed-off-by: David Nind <david@davidnind.com>

Hi :) It seems to work, does anyone know where it's possible to find if a hold cancellation reason was taken into account? Without setting up emails to check the notification sent to the patron.

About the UI, here is the before and after:
https://pic.infini.fr/zdxkpN6K/WpXYQEQ5.png
https://pic.infini.fr/5qEfKaF5/f5RWTWAY.png

It seemed messy to me on two lines and all on the right. And someone else from QA told me it's a bit all over the place. I'm continuing to get feedback to know if that's good enough or not and **how** it should be changed.

Just noticed that there is more padding with the lower edge of the window now.

There definitely needs space for the dropdown because the authorized value could be long and there are language more verbose on average than english. So it's normal it can't fit with the rest.

Is there a similar place in Koha to compare?

> I'm continuing to get feedback to know if that's good enough or not and **how** it should be changed.


Got another feedback about "the text above being left-aligned and the buttons right-aligned". So maybe it's mostly that that looks off.

I noticed this with testing, but didn't add any comment (as it worked).

Not sure if I've seen this pattern before anywhere else, and I don't have any suggestions on how to improve the display...

(In reply to Victor Grousset/tuxayo from comment #4)
> About the UI, here is the before and after:
> https://pic.infini.fr/zdxkpN6K/WpXYQEQ5.png
> https://pic.infini.fr/5qEfKaF5/f5RWTWAY.png
> 
> It seemed messy to me on two lines and all on the right. And someone else
> from QA told me it's a bit all over the place. I'm continuing to get
> feedback to know if that's good enough or not and **how** it should be
> changed.
> 
> Just noticed that there is more padding with the lower edge of the window
> now.
> 
> There definitely needs space for the dropdown because the authorized value
> could be long and there are language more verbose on average than english.
> So it's normal it can't fit with the rest.
> 
> Is there a similar place in Koha to compare?

Hi Victor,

So the input comes from comes from the holds tab on patron details view, I delete the label element. So I put it like this on a very simple way but if the design needs to be reworked, I'd be happy to hear from you. 

https://pic.infini.fr/uVZne6iG/bEcWnqRM.png

> > Is there a similar place in Koha to compare?

> So the input comes from comes from the holds tab on patron details view
> [...]
> https://pic.infini.fr/uVZne6iG/bEcWnqRM.png

By similar place I meant a modal with the bottom starting to get crowded and needing some though in the placement of elements.

----

> So I put it like this on a very simple way but if the design needs to be reworked, I'd be happy to hear from you. 

I'll reach out for advice to someone more knowledgeable about UI.

Maybe while I'm letting here two attempts of change with browser dev tools. (got lucky to find the CSS for alignment)
left centered: https://pic.infini.fr/bWRu0B0L/CFKSoX4L.png
left centered & swapped button order:  https://pic.infini.fr/bNuJzlXG/oCY2D3EM.png

---

(In reply to Victor Grousset/tuxayo from comment #3)
> Hi :) It seems to work, does anyone know where it's possible to find if a
> hold cancellation reason was taken into account? Without setting up emails
> to check the notification sent to the patron.

About this we can use the sandboxes: https://wiki.koha-community.org/wiki/Sandboxes#Testing_sending_emails

But they don't work for now (for some reason they are late on Koha commit and bug 35295 block testing this very bug)

 WARN   circ/returns.pl
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 225, now: 227)

Created attachment 158794 [details] [review]
Bug 29007: Add cancellation reason input on check in

Test plan:

1) Check an item in with already a confirmed hold
2) Cancel the hold, it was cancelled without possibility to choosing a
   reason
3) Apply this patch and do it again, you must see an input to choose
   your cancellation reason (list from VA)

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Pushed to master for 23.11.

Nice work everyone, thanks!

Thanks Marcel for validating that it's good enough.

Pushed to 23.05.x for 23.05.06

This documentation is done and in the manual.