Bug 9824 - Hide basket with no expected items in basket list by bookseller
Summary: Hide basket with no expected items in basket list by bookseller
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Koha Team University Lyon 3
QA Contact: Marcel de Rooy
URL:
Keywords:
: 8611 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-03-15 17:02 UTC by Koha Team University Lyon 3
Modified: 2014-12-07 20:02 UTC (History)
13 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
change sql query in GetBasketsInfosByBookseller (867 bytes, patch)
2013-03-15 17:19 UTC, Koha Team University Lyon 3
Details | Diff | Splinter Review
Hide-basket-with-no-expected-items-in-basqket (920 bytes, patch)
2013-03-21 14:16 UTC, Koha Team University Lyon 3
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9824 - Hide basket with no expected items in basqket list by bookseller (1.38 KB, patch)
2013-05-16 13:05 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 9824 - Hide basket with no expected items in basqket list by bookseller (1.42 KB, patch)
2013-05-17 09:33 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9824: Followup for removing filter (3.86 KB, patch)
2013-05-17 17:11 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9824: Followup for removing filter (3.80 KB, patch)
2013-05-17 17:28 UTC, Marcel de Rooy
Details | Diff | Splinter Review
[SIGNED OFF] Bug 9824: Followup for removing filter (4.09 KB, patch)
2013-05-17 19:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 9824: Make it possible to apply/cancel switch (1.92 KB, patch)
2013-05-17 19:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9824: Make it possible to apply/cancel switch (2.03 KB, patch)
2013-05-17 19:27 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 9824: Followup for POD lines of GetBasketsInfosByBookseller (1.10 KB, patch)
2013-05-22 13:01 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9824: Followup adding some text around Apply/cancel filter. (1.72 KB, patch)
2013-05-22 13:08 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9824: Followup adding some text around Apply/cancel filter. (1.85 KB, patch)
2013-05-22 13:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
[SIGNED_OFF] Bug 9824: Followup for POD lines of GetBasketsInfosByBookseller (1.11 KB, patch)
2013-05-23 08:51 UTC, Paul Poulain
Details | Diff | Splinter Review
Bug 9824: Followup adding some text around Apply/cancel filter. (1.86 KB, patch)
2013-05-23 09:01 UTC, Paul Poulain
Details | Diff | Splinter Review
[NEEDS_SIGNOFF] Bug 9824: Followup adding some text around Apply/cancel filter. (1.80 KB, patch)
2013-05-23 12:22 UTC, Marcel de Rooy
Details | Diff | Splinter Review
[SIGNED_OFF] Bug 9824: Followup adding some text around Apply/cancel filter. (1.81 KB, patch)
2013-05-30 09:45 UTC, Paul Poulain
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Koha Team University Lyon 3 2013-03-15 17:02:59 UTC
The baskets list is growing year and year and it very difficult to see baskets where items are in command and expected. The page can be very long to load.
Only the basket with expected items should be list in this page.
Comment 1 Koha Team University Lyon 3 2013-03-15 17:19:20 UTC Comment hidden (obsolete)
Comment 2 Koha Team University Lyon 3 2013-03-15 17:24:41 UTC
Before patch you will see in acqui/booksellers.pl all the baskets ever created.
After the patch you will see in acqui/booksellers.pl only the basket with expected items.

Test plan :
* Create a basket with some orders lines
You should see this basket in acqui/booksellers.pl
* receive or cancel all the line in this basket
This basket shouldn't appear any more in acqui/booksellers.pl
Comment 3 Mathieu Saby 2013-03-15 22:16:57 UTC
Hello Lyon3
I did not test your patch yet, but a solution to "death by overbasketing" could also be hidding baskets linked to non active budgect.
And the same could be done for basketgroups too.
What is your opinion ?

M. Saby
Comment 4 Blain Laurence 2013-03-18 11:35:10 UTC
After receiving all items, we still see the basket in booksellers.pl.
Comment 5 Blain Laurence 2013-03-18 11:37:03 UTC
tested on sandbox biblibre
Comment 6 Koha Team University Lyon 3 2013-03-21 14:16:26 UTC Comment hidden (obsolete)
Comment 7 Koha Team University Lyon 3 2013-03-21 14:26:45 UTC
In reply of comment 3 : Mathieu I don't understand your "death by overbasketing" (we should talk about this yesterday...)
For basketgroups, we can do another patch...
Sonia
Comment 8 Pierre Angot 2013-03-22 15:57:05 UTC
It works. But how can you find  a basket ?
Comment 9 Katrin Fischer 2013-03-22 15:59:06 UTC
You can search for baskets using the order search.
Comment 10 Katrin Fischer 2013-03-22 16:01:04 UTC
I had no time looking for it, but I think this is actually fixing a regression. It used to be so that you won't see any baskets that had been fully received in the vendor result list. Then that got changed again. I am thinking this could be a bug fix, at least for libraries coming from 3.6 and maybe a bit later.
Comment 11 Pierre Angot 2013-03-23 11:41:19 UTC
I'm anticipating some questions from our librarian colleagues. They might ask us how to see all baskets from a vendor. I think it would be difficult for them to use acquisition module.
If all receveid baskets disappear from result list, it is good (cf desciption). But, with this patch, we can't see any baskets when we are on a vendor page.
Comment 12 Koha Team University Lyon 3 2013-03-26 10:05:13 UTC
It's always booksellers.pl which is called whatever you go in result list or you choose "baskets" in vendor page.
In order search, you can search by vendor and you will find all the orders with the name of basket...
The BZ9806 will permit to see the baskets in each basket group : it's an other way to find baskets from a vendor.
Comment 13 Sophie MEYNIEUX 2013-03-26 14:14:10 UTC
*** Bug 8611 has been marked as a duplicate of this bug. ***
Comment 14 Paul Poulain 2013-05-03 15:13:00 UTC
I think there's something wrong with attachments:
the #2 is "octet-stream", it should be a patch I think.
isn't the #1 obsoleted by the #2 too ?

To answer the main question : I think it's more a bugfix than an ENH : baskets that are 100% received are supposed not to be displayed anymore on this page.

Side comment: we must display baskets that are not closed, whatever the # of items still to receive. Otherwise, empty baskets will disappear.
Comment 15 Owen Leonard 2013-05-09 14:25:32 UTC
This patch doesn't apply for me through git-bz, but did apply using "git am -i3u" after saving the patch. I just wanted to mention it if others are testing too.

It works correctly to hide baskets with no items to be received, but I have one worry: Are librarians going to be confused about how to access those baskets? Should there be a "show all" link or something like that?
Comment 16 Owen Leonard 2013-05-09 14:29:47 UTC
(In reply to comment #11)
> I'm anticipating some questions from our librarian colleagues. They might
> ask us how to see all baskets from a vendor. I think it would be difficult
> for them to use acquisition module.

Whoops, I see someone else has asked this already! I'm not a regular acquisitions user, so it's hard for me to say, however, I would think a "show all" link would be useful? Anyone else?
Comment 17 Katrin Fischer 2013-05-09 16:12:46 UTC
Hi Owen,

I'd really like to see the regression here fixed so that we are only showing baskets with outstanding orders by default, but a link to show more would certainly not hurt. Maybe we could just link into the aquisition search?
Comment 18 Katrin Fischer 2013-05-11 12:29:51 UTC
Hi Owen, reading your comments I see you tested this - is this ready for sign off or is there a problem?
Comment 19 Mathieu Saby 2013-05-13 07:25:54 UTC
In my opinion,
Comment 20 Mathieu Saby 2013-05-13 08:01:05 UTC
Sorry, Firefox is playing with me this morning...

So, in my opinion, the goal of Lyon 3 patch is good - showing only useful information in basket lists - , but I am not sure to like the way it is working.

The patch changes the default behavior GetBasketsInfosByBookseller function in Acquisition.pm. Would not it be better to maintain its default behavior, and to add an option if we want to filter the results ? (and in that case, the documentation of the function needs to be changed too).
Maybe we could also imagine a way for this function to not get by default baskets with documents only linked with non active budgets ?

But I think I would prefer keeping the function unchanged, and only hide received baskets in the templates. It will make possible the creation of a link "See also received basket" that would unhide received baskets. And I agree with Pierre : we must have a way to show all baskets if we want.
In templates, we could have checkboxes or option buttons to see received/pending/closed/open baskets, according to the whish of the librarians.

M. Saby
Comment 21 Katrin Fischer 2013-05-15 09:19:39 UTC
Hm I have done some research. The change discussed here was made when the result list for vendors was redone and GetBasketsInfosByBookseller was introduced (http://git.koha-community.org/gitweb/?p=koha.git;a=commit;h=d09e428a7dc9854c6f4e45f3caade3c8e141b79d):

The old list of baskets was created using following condition:
-where booksellerid = ?
-AND ( aqorders.quantity > aqorders.quantityreceived OR quantityreceived IS NULL)
-AND datecancellationprinted IS NULL

The new sub uses:
+        WHERE booksellerid = ?

The change proposed in the patch just restores the original condition:
+        WHERE booksellerid = ? 
AND ( aqorders.quantity > aqorders.quantityreceived OR quantityreceived IS NULL)
+         AND datecancellationprinted IS NULL

I know that the patch was pushed a while ago, but since we are still using 3.6.x in some libraries, this means a major change for them. Is there a chance we can agree on a solution here?

I am in favor of restoring the original behaviour for now and using the search to find other older baskets.
Comment 22 Owen Leonard 2013-05-16 13:05:54 UTC Comment hidden (obsolete)
Comment 23 Marcel de Rooy 2013-05-17 09:02:38 UTC
QA: Looking at this one now..
Comment 24 Marcel de Rooy 2013-05-17 09:32:50 UTC
QA Comment:
Works fine. My only question was: How to find back this basket if it is no longer on that list. But with Order search or its Advanced Search, you can still find it.
Small patch. No complaints from qa-tools. Looks good.

BTW: I do not think that quantityreceived could be NULL (constrained), but this test does not hurt either.

Final thought: Perhaps another improvement could be: always show baskets that are still open and apply your condition only on the closed ones..
If you agree on that, could you add a followup?

Passed QA
Comment 25 Marcel de Rooy 2013-05-17 09:33:14 UTC
Created attachment 18184 [details] [review]
Bug 9824 - Hide basket with no expected items in basqket list by bookseller

Before patch you will see in acqui/booksellers.pl all the baskets ever
created.
After the patch you will see in acqui/booksellers.pl only the basket
with expected items.

Test plan :
* Create a basket with some orders lines
You should see this basket in acqui/booksellers.pl
* receive or cancel all the line in this basket
This basket shouldn't appear any more in acqui/booksellers.pl

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 26 Jared Camins-Esakov 2013-05-17 12:03:53 UTC
This patch has been pushed to master and 3.12.x.

I have serious reservations about this patch, but am pushing it anyway as it simply reverts Koha to its previous behavior, problematic as that was. With this patch, I am no longer able to get an overview of baskets for a particular vendor. I can see a list of orders using the Orders search, but there's no overview of baskets available. I am pushing this only because I trust that someone will fix this issue properly for the 3.12.1 release.
Comment 27 Katrin Fischer 2013-05-17 12:14:23 UTC
I have been thinking about this for a while now and I can see that it's problematic because the old behaviour has been broken quite a while ago and now it could be a problematic change for other libraries using Koha's acq. 

I was wondering if I could use jQuery to 'revert' to the original behaviour... probably a bit tricky, but it could work until we see this fixed properly.
Comment 28 Mathieu Saby 2013-05-17 12:16:01 UTC
For 3.12.1, what about a filter on the left of the page, giving ability to choose to display received/pending/closed/open baskets, according to the whish of the librarians ?

M. Saby
Comment 29 Katrin Fischer 2013-05-17 12:21:13 UTC
That sounds nice - as long as we get something. I think having the shorter list (limited to 'unfinished' baskets) might be a good default, but have an option to show all/pending/open/closed baskets would certainly be nice.
Comment 30 Jared Camins-Esakov 2013-05-17 12:25:03 UTC
(In reply to comment #27)
> I have been thinking about this for a while now and I can see that it's
> problematic because the old behaviour has been broken quite a while ago and
> now it could be a problematic change for other libraries using Koha's acq. 
> 
> I was wondering if I could use jQuery to 'revert' to the original
> behaviour... probably a bit tricky, but it could work until we see this
> fixed properly.

That would be quite easy to do:
$('.baskets tbody tr').each(function () {
    if ($('td:eq(2)', this).text() == $('td:eq(4)', this).text() && $('td:eq(2)', this).text() != '0') {
        $(this).hide();
    }
});

It sounds like a better patch is in the works for 3.12.1, so I am inclined to revert this patch for now, as it will change the behavior for anyone already using 3.8 or 3.10, and no one who upgraded from 3.6 noticed the change until this March. I will delay a decision until later today.
Comment 31 Paul Poulain 2013-05-17 12:26:21 UTC
(In reply to comment #26)
> This patch has been pushed to master and 3.12.x.
> 
> I have serious reservations about this patch, but am pushing it anyway as it
> simply reverts Koha to its previous behavior, problematic as that was. With
> this patch, I am no longer able to get an overview of baskets for a
> particular vendor. I can see a list of orders using the Orders search, but
> there's no overview of baskets available. I am pushing this only because I
> trust that someone will fix this issue properly for the 3.12.1 release.

Am I missing something ?
Koha > Acquisitions > Order search > Advanced search > enter basket # in basket field, and you'll get all the content of basket # !
Doesn't that fit your need ?

Which use case could achieve a list of baskets for a particular vendor ? I don't see one (but I'm OK to be convinced being wrong ;-) )
Comment 32 Jared Camins-Esakov 2013-05-17 12:31:24 UTC
(In reply to comment #31)
> (In reply to comment #26)
> Am I missing something ?
> Koha > Acquisitions > Order search > Advanced search > enter basket # in
> basket field, and you'll get all the content of basket # !
> Doesn't that fit your need ?

Not smoothly.

> Which use case could achieve a list of baskets for a particular vendor ? I
> don't see one (but I'm OK to be convinced being wrong ;-) )

I have a library that uses Acquisitions to record gifts, and each gift is a separate basket, and with a list of baskets they can review how often/when they received gifts, without needing to slog through lists of items.
Comment 33 Koha Team University Lyon 3 2013-05-17 12:37:17 UTC
"no one who upgraded from 3.6 noticed the change until this March"
I can say that my colleagues noticed and come to complain very often !!
I know that Lyon2 university had to change all the vendor every year to get around this problem.
I take account of the desire of people who manage the acquisitions in my university, but if operations differ in other organizations, i can understand that we try to giving ability to choose to display received/pending/closed/open baskets.
What is the need of people that really deals with acquisitions ?
Comment 34 Koha Team University Lyon 3 2013-05-17 12:41:59 UTC
I'm sorry but using acquistions module for gift is a sort of diversion, no ?
If this change is introduced not for the proper use, i'm not sure it's a good idea..
Sonia
Comment 35 Mathieu Saby 2013-05-17 12:46:22 UTC
In Rennes 2, we agree the default behavior MUST be changed, and we are fine with this patch being pushed in master.

The only remark is that MAYBE it could be useful for a lib, or the chief librarian responsible for acquisitions to see all baskets. But not everyday ;-)
And we could add new possibilities for filtering and sorting baskets (for ex. sorting by date, showing only open baskets etc).

Mathieu
Comment 36 Marcel de Rooy 2013-05-17 15:41:49 UTC
Default should probably include the open baskets too, that could be excluded in the current change (as noted in the QA comment) if they have no order lines or if it is reopened but everything is received.

A simple link to remove this default filter (and thus show all) might be a way to prevent the need of reverting this patch?
Comment 37 Marcel de Rooy 2013-05-17 17:11:42 UTC Comment hidden (obsolete)
Comment 38 Marcel de Rooy 2013-05-17 17:13:10 UTC
This *trick* would not really involve a string change (for emphasis).
Comment 39 Jared Camins-Esakov 2013-05-17 17:17:02 UTC
I would be satisfied with that change.
Comment 40 Marcel de Rooy 2013-05-17 17:24:42 UTC
Still found a little issue with my change: sometimes supplier contains the search string, but sometimes the bookseller id numeric already.

Will have a quick look :)
Comment 41 Marcel de Rooy 2013-05-17 17:28:49 UTC Comment hidden (obsolete)
Comment 42 Marcel de Rooy 2013-05-17 17:29:23 UTC
(In reply to comment #40)
> Still found a little issue with my change: sometimes supplier contains the
> search string, but sometimes the bookseller id numeric already.

Resolved: added both ways in the url.
Comment 43 Marcel de Rooy 2013-05-17 17:42:51 UTC
My last action for today on Bugzilla will be: eliminate this typo basqket in the title of this great report :)
Comment 44 Katrin Fischer 2013-05-17 18:41:56 UTC
Good news - you really tricked the translation scripts!
Now it would be perfect if we could make a link 'Apply filter' show up after clicking the link for the first time... looking into it.
Comment 45 Katrin Fischer 2013-05-17 19:18:39 UTC
Created attachment 18219 [details] [review]
[SIGNED OFF] Bug 9824: Followup for removing filter

Adds allbaskets parameter to GetBasketsInfosByBookseller. (Only used in booksellers.pl now)
Normally, all 'active' baskets are shown. With allbaskets=1 all baskets :)
In the template I had to rename a loop var supplier to supplier1 to resolve
name conflict between template vars.
In the template I added the string: Cancel filter.
Note that this string is already translated:
msgid "Cancel filter"
msgstr ""
Hope this helps.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Undoing the filter works and I checked that the string gets
translated with the po files in current master.
So this is almost perfect, only we can't apply the filters
again and the link remains 'cancel' when we already did.
Sending a follow-up trying to fix this.
Comment 46 Katrin Fischer 2013-05-17 19:18:54 UTC Comment hidden (obsolete)
Comment 47 Owen Leonard 2013-05-17 19:27:17 UTC
Created attachment 18221 [details] [review]
[SIGNED-OFF] Bug 9824: Make it possible to apply/cancel switch

Using another existing string 'Apply filter' we are now
able to cancel the filter... and apply it... and cancel it
again... and so on.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 48 Jared Camins-Esakov 2013-05-19 12:44:23 UTC
Exercising my powers as RM and passing QA since the patches work.
Comment 49 Jared Camins-Esakov 2013-05-19 12:44:34 UTC
This patch has been pushed to master and 3.12.x.
Comment 50 Marcel de Rooy 2013-05-22 13:01:07 UTC Comment hidden (obsolete)
Comment 51 Marcel de Rooy 2013-05-22 13:08:34 UTC Comment hidden (obsolete)
Comment 52 Marcel de Rooy 2013-05-22 13:37:13 UTC Comment hidden (obsolete)
Comment 53 Marcel de Rooy 2013-05-22 13:39:53 UTC
Katrin: Do you still have a suggestion on the second followup for textual changes? Perhaps from translation point of view. I submitted an amendment to make translation somewhat easier.

Status back to Signed off for two trivial QA followups.
Comment 54 Marcel de Rooy 2013-05-23 07:38:01 UTC
As requested, back to Needs signoff.
Could a native speaker look at the proposed textual change?
Thanks.
Comment 55 cedric.vita 2013-05-23 08:03:21 UTC
Sandbox setup by unknown with database 2 and bug 9824 on Thu May 23 09:59:56 2013

Something went wrong !
Applying: Bug 9824: Followup for POD lines of GetBasketsInfosByBookseller
Applying: Bug 9824: Followup adding some text around Apply/cancel filter.
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/booksellers.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/acqui/booksellers.tt
Failed to merge in the changes.
Patch failed at 0001 Bug 9824: Followup adding some text around Apply/cancel filter.
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
Bug 9824 - Hide basket with no expected items in basket list by bookseller

Bug 9824: Followup for POD lines of GetBasketsInfosByBookseller
Apply? [yn]
Bug 9824: Followup adding some text around Apply/cancel filter.
Apply? [yn]
Patch left in /tmp/Bug-9824-Followup-adding-some-text-around-Applycan-myQQFX.patch
Comment 56 Marcel de Rooy 2013-05-23 08:13:40 UTC
(In reply to comment #55)
> Sandbox setup by unknown with database 2 and bug 9824 on Thu May 23 09:59:56
> 2013
> 
> Patch failed at 0001 Bug 9824: Followup adding some text around Apply/cancel
> filter.

Thanks for testing!
Strange. These two patches do apply fine for me on current master.
Are you sure that the sandbox setup is on current master?

Back to Needs Signoff
Comment 57 Paul Poulain 2013-05-23 08:51:16 UTC
Patch tested with a sandbox, by Cedric Vita <cedric.vita@dracenie.com>
Comment 58 Paul Poulain 2013-05-23 08:51:34 UTC Comment hidden (obsolete)
Comment 59 cedric.vita 2013-05-23 08:59:31 UTC
Error sandbox for sinagture. I will revalidate the good "Pelardon"
Comment 60 Paul Poulain 2013-05-23 09:01:19 UTC
Patch tested with a sandbox, by Cedric Vita <cedric.vita@dracenie.com>
Comment 61 Paul Poulain 2013-05-23 09:01:41 UTC Comment hidden (obsolete)
Comment 62 Marcel de Rooy 2013-05-23 12:22:04 UTC Comment hidden (obsolete)
Comment 63 Marcel de Rooy 2013-05-23 12:23:16 UTC
Thank you for your quick signoff, cedric vita.
I reworded the text of the links on the second patch, with thanks to Owen.
Comment 64 Mathieu Saby 2013-05-25 13:50:57 UTC
The status was not changed. Could be marked as "Signed off" I suppose?

M. Saby
Comment 65 Katrin Fischer 2013-05-25 14:32:56 UTC
I think probably the reworded patch waits for sign-off.
Comment 66 Mathieu Saby 2013-05-25 15:25:27 UTC
ok, i'll test it.
but there is only 2 followups. Where is the main patch?

Mathieu
Comment 67 Chris Cormack 2013-05-26 06:22:17 UTC
(In reply to comment #66)
> ok, i'll test it.
> but there is only 2 followups. Where is the main patch?
> 
> Mathieu

It was pushed to master and 3.12 way back on comment 49, at that point a new bug should have been created for any followups, because as you demonstrate, it is just confusing otherwise.

Remember people, once something is marked pushed to master, please to go back to needs signoff, (unless it is the patch rebased for a stable branch or something).

Thanks
Comment 68 Marcel de Rooy 2013-05-27 06:34:13 UTC
(In reply to comment #66)
> but there is only 2 followups. Where is the main patch?

The second followup only needs a signoff, preferably from a native English speaker. (Note that I already followed the suggestion of Owen here.)
Thanks.
Comment 69 Marcel de Rooy 2013-05-27 06:35:15 UTC
(In reply to comment #67)
> Remember people, once something is marked pushed to master, please to go
> back to needs signoff, (unless it is the patch rebased for a stable branch
> or something).

Not completely clear what you mean here, Chris. But these followups imo do not need a separate report..
Comment 70 Katrin Fischer 2013-05-27 06:42:48 UTC
Hm, I was tempted to suggest a separate bug too when I took a look at this. It seems like a good practice to me to use a new bug once the original feature has been pushed. And in this case the original feature has even been released :) - I am also thinking about the automatically created release notes here. Maybe something like "Reword basket filter in vendor search result list" would be more clear?

For testers and QA it's also harder to deal with a loooong discussion like this :)
Comment 71 Mathieu Saby 2013-05-27 07:30:43 UTC
(En réponse au commentaire 70)
> Hm, I was tempted to suggest a separate bug too when I took a look at this.
> It seems like a good practice to me to use a new bug once the original
> feature has been pushed. And in this case the original feature has even been
> released :) - I am also thinking about the automatically created release
> notes here. Maybe something like "Reword basket filter in vendor search
> result list" would be more clear?
> 
> For testers and QA it's also harder to deal with a loooong discussion like
> this :)

I don't know if it is already an enforced "rule", but 1 or 2 times I tried to add followup to bugs pushed to master, and I was said to create new bugs (for ex., a followup for UNIMARC of a MARC21 enh).
After having thougth at this, I believe it was a good advice ;-)

M. Saby
Comment 72 Marcel de Rooy 2013-05-27 07:57:40 UTC
Sorry, will try to remember your suggestion next time :)
Katrin: As you suggested, I asked Owen on irc and adjusted the text. Would you mind passing these two patches on this report now or still require a new one?
Thanks.
Comment 73 Paul Poulain 2013-05-30 09:45:23 UTC
Patch tested with a sandbox, by Pierre Angot <tredok.pierre@gmail.com>
Comment 74 Paul Poulain 2013-05-30 09:45:42 UTC Comment hidden (obsolete)
Comment 75 Jonathan Druart 2013-05-30 14:24:41 UTC
All 3 patches of this bug have been pushed to master and 3.12 branches
Comment 76 Marcel de Rooy 2013-05-30 18:45:24 UTC
(In reply to comment #75)
> All 3 patches of this bug have been pushed to master and 3.12 branches

No, this is not true. Somehow you obsoleted the two followups.
I will try to recover them now.
Comment 77 Marcel de Rooy 2013-05-30 18:55:01 UTC
The two followups 18332 and 18506 are now signed off. They apply fine to current master.
Although I wrote them, they are actually QA followups on the three patches pushed before.
Since they only pertain to a small textual change and some POD lines, I am setting the status to Passed QA myself, herewith ending the confusion about the status of this report.

Passed QA
Comment 78 Marcel de Rooy 2013-05-30 18:57:13 UTC
(In reply to comment #76)
> (In reply to comment #75)
> > All 3 patches of this bug have been pushed to master and 3.12 branches
> 
> No, this is not true. Somehow you obsoleted the two followups.
> I will try to recover them now.

And now I see the move of these patches to another report, aaaargggghhh.
OK. Please add a comment if you do so.
Will obsolete everything again..
Comment 79 Marcel de Rooy 2013-05-30 19:13:19 UTC
Restored the situation now where Jonathan left it.
Learned something more about Bugzilla :)
The two followups on these three reports are on bug 10381.
Comment 80 Marcel de Rooy 2013-05-30 19:18:29 UTC
(In reply to comment #79)
> The two followups on these three reports are on bug 10381.
reports -> patches

No confusion anymore now?
Comment 81 Jonathan Druart 2013-05-31 07:27:17 UTC
Sorry Marcel, I should have add a comment in addition to the "see also" field.
Comment 82 Marcel de Rooy 2013-05-31 09:13:37 UTC
(In reply to comment #81)
> Sorry Marcel, I should have add a comment in addition to the "see also"
> field.

No problem. Just looking a little bit better could have helped me :)
Comment 83 Pierre Angot 2013-06-05 13:11:59 UTC
Is it possible to push this patch to 3.10.x ?
Comment 84 Bernardo Gonzalez Kriegel 2013-06-14 01:31:38 UTC
Pushed to 3.10.x, will be in 3.10.7
Comment 85 Chris Hall 2013-06-17 07:52:08 UTC
Pushed to 3.8.x, will be in 3.8.14
Comment 86 Pierre Angot 2013-06-22 08:08:17 UTC
(En réponse au commentaire 84)
> Pushed to 3.10.x, will be in 3.10.7

Thanks !