Bug 21048

Created attachment 76745 [details]
No status when viewing suggestion

Created attachment 76746 [details]
Visible when filtering suggestions

Created attachment 76747 [details]
OPAC View

6. The OPAC side shows the authorized value instead of the OPAC description.

Created attachment 76872 [details] [review]
Bug 21048: Fix display of suggestion's status (SUGGEST_STATUS)

When a suggestion's status is set to a customized value (AV
SUGGEST_STATUS), the description is not always displayed correctly.

Test plan:
- Create different values for the AV SUGGEST_STATUS
- Create several suggestions
- Change the status using the SUGGEST_STATUS values
- Edit the suggestion and change again the status
- View the suggestions for a given patron
(members/purchase-suggestions.pl?borrowernumber=XX), the statuses must
displayed correctly

I only have recreated the first issue (and find another one when editing a suggestion). The tab displayed ok for me. Which exact version of Koha are you using?

On master I see the tabs which have status with suggestions. Emtpy tabs are not displayed.

We are on 17.11.07.000. 
On master on Sandbox It shows a bunch of tabs labeled "Status unknown (0)' and a tab for whichever ones have at least 1 purchase suggestion with that value.

Patch tested with a sandbox, by Lisette Scheer <lisetteslatah@gmail.com>

Created attachment 76900 [details] [review]
Bug 21048: Fix display of suggestion's status (SUGGEST_STATUS)

When a suggestion's status is set to a customized value (AV
SUGGEST_STATUS), the description is not always displayed correctly.

Test plan:
- Create different values for the AV SUGGEST_STATUS
- Create several suggestions
- Change the status using the SUGGEST_STATUS values
- Edit the suggestion and change again the status
- View the suggestions for a given patron
(members/purchase-suggestions.pl?borrowernumber=XX), the statuses must
displayed correctly

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

(In reply to Lisette Scheer from comment #6)
> We are on 17.11.07.000. 
> On master on Sandbox It shows a bunch of tabs labeled "Status unknown (0)'
> and a tab for whichever ones have at least 1 purchase suggestion with that
> value.

How did you do that? :)
I have created a new suggestion on another sandbox (5) and there is only one tab!

Created attachment 76925 [details]
Multiple Status Unknown tabs

This was on BibLibre sandbox 06 both on master and with your patch applied.
I created 3 suggest_status AV, made 3 purchase suggestions and assigned one to 2 of the statuses and left one pending. Each time it added a new tab, but left the status unknown tabs visible. 

There were the same number of status unknown tabs when I had 1 AV as when I had 3.  

I tried on BibLibre sandbox 07 on master and before I even added any suggest_status AV I put in a purchase suggestion and it did this again. 

Perhaps it is an issue with the BibLibre sandboxes? I'd try on another but those are the only ones I've been able to get to work today. 
Lisette

Line 680 still contains:
[% IF s.authorised_value == selected_status %]
Around line 308 you are replacing it by suggestion.STATUS. Please explain. If I am not mistaken, it comes from: my $suggestion_ref  = $input->Vars; Where is the field STATUS filled?

See the following warn when I haved added an additional status and I am saving a suggestion with that status.
No suggestions TEST3 letter transported by email at /usr/share/koha/devclone/C4/Letters.pm line 602.

(In reply to Marcel de Rooy from comment #11)
> Line 680 still contains:
> [% IF s.authorised_value == selected_status %]
> Around line 308 you are replacing it by suggestion.STATUS. Please explain.
> If I am not mistaken, it comes from: my $suggestion_ref  = $input->Vars;
> Where is the field STATUS filled?

361     suggestion   => $suggestion_ref,

> See the following warn when I haved added an additional status and I am
> saving a suggestion with that status.
> No suggestions TEST3 letter transported by email at
> /usr/share/koha/devclone/C4/Letters.pm line 602.

I guess this existed prior to this change. It looks like a configuration issue, or an expected behavior, ModSuggestion is sending an email to the author of the suggestion when the status is changed. The notice template used is the one with the same code as the status (that is pretty bad, it should at least be prefixed by something). I would not consider this as blocker for this bugfix/

(In reply to Lisette Scheer from comment #10)
> Created attachment 76925 [details]
> Multiple Status Unknown tabs

This is very weird. I do not have access to sandbox and cannot recreate locally so it's hard help.

Hi Lisette, 
can you share how you set up your SUGGEST_STATUS values?

(In reply to Jonathan Druart from comment #12)
> (In reply to Marcel de Rooy from comment #11)
> > Line 680 still contains:
> > [% IF s.authorised_value == selected_status %]
> > Around line 308 you are replacing it by suggestion.STATUS. Please explain.
> > If I am not mistaken, it comes from: my $suggestion_ref  = $input->Vars;
> > Where is the field STATUS filled?
> 
> 361     suggestion   => $suggestion_ref,
OK. But what about line 680? The selected_status there is not filled anywhere?

> > See the following warn when I haved added an additional status and I am
> > saving a suggestion with that status.
> > No suggestions TEST3 letter transported by email at
> > /usr/share/koha/devclone/C4/Letters.pm line 602.
> 
> I guess this existed prior to this change. It looks like a configuration
> issue, or an expected behavior, ModSuggestion is sending an email to the
> author of the suggestion when the status is changed. The notice template
> used is the one with the same code as the status (that is pretty bad, it
> should at least be prefixed by something). I would not consider this as
> blocker for this bugfix/
No, it is certainly not a blocker here.

(In reply to Marcel de Rooy from comment #15)
> (In reply to Jonathan Druart from comment #12)
> > (In reply to Marcel de Rooy from comment #11)
> > > Line 680 still contains:
> > > [% IF s.authorised_value == selected_status %]
> > > Around line 308 you are replacing it by suggestion.STATUS. Please explain.
> > > If I am not mistaken, it comes from: my $suggestion_ref  = $input->Vars;
> > > Where is the field STATUS filled?
> > 
> > 361     suggestion   => $suggestion_ref,
> OK. But what about line 680? The selected_status there is not filled
> anywhere?

It sounds like a bug but I cannot recreate from the UI, maybe this is never used?
Could you have a look?

(In reply to Katrin Fischer from comment #14)
> Hi Lisette, 
> can you share how you set up your SUGGEST_STATUS values?

The ones on the Valnet system were already set up when I got here. 
On the sandboxes I tested on here are my steps:
Go to Administration
Go to Authorized Values
Select 'SUGGEST_STATUS'
Click the "New Value for SUGGEST_STATUS" button. 
Type in an unique AV (doesn't match any other AV in SUGGEST_STATUS or other AVs)
Type in a description and OPAC description
Click save. 

Lisette

(In reply to Jonathan Druart from comment #16)
> (In reply to Marcel de Rooy from comment #15)
> > (In reply to Jonathan Druart from comment #12)
> > > (In reply to Marcel de Rooy from comment #11)
> > > > Line 680 still contains:
> > > > [% IF s.authorised_value == selected_status %]
> > > > Around line 308 you are replacing it by suggestion.STATUS. Please explain.
> > > > If I am not mistaken, it comes from: my $suggestion_ref  = $input->Vars;
> > > > Where is the field STATUS filled?
> > > 
> > > 361     suggestion   => $suggestion_ref,
> > OK. But what about line 680? The selected_status there is not filled
> > anywhere?
> 
> It sounds like a bug but I cannot recreate from the UI, maybe this is never
> used?
> Could you have a look?

It is a very small bug. I will fill selected_status in the script instead of suggestion.STATUS since the template uses suggestion here as a loop variable.

Created attachment 77298 [details] [review]
Bug 21048: Fix display of suggestion's status (SUGGEST_STATUS)

When a suggestion's status is set to a customized value (AV
SUGGEST_STATUS), the description is not always displayed correctly.

Test plan:
- Create different values for the AV SUGGEST_STATUS
- Create several suggestions
- Change the status using the SUGGEST_STATUS values
- Edit the suggestion and change again the status
- View the suggestions for a given patron
(members/purchase-suggestions.pl?borrowernumber=XX), the statuses must
displayed correctly

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

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

Created attachment 77299 [details] [review]
Bug 21048: (QA follow-up) Fix authorized value statuses in filter

If you select an authorized value status in the filter, you want to keep
it too just like normal statuses as CHECKED.
The variable selected_status was not filled. The minimal fix is adding that
template variable in the script. Note that suggestion.STATUS is out of scope
within a loop using suggestion as loop var.

Note: Adding a regular status like CHECKED as an authorized value with
same code but another description works, but is kind of confusing ;) Not
in the scope of this report though.

Test plan:
Select an authval status in the filter. Apply the filter and verify again
that it is still selected under Suggestion information.

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

Awesome work all!

Pushed to master for 18.11

Pushed to 18.05.x for 18.05.03

Pushed to 17.11.x for 17.11.10