| Summary: | Unable to delete a subscription | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Alex Arnaud <alex.arnaud> |
| Component: | Serials | Assignee: | Jonathan Druart <jonathan.druart> |
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | colin.campbell, fridolin.somers, jonathan.druart, m.de.rooy, martin.renvoize, nick, sandboxes, severine.queune |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Trivial patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Bug Depends on: | 19777 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 20778: Fix subscription deletion. Also remove the first confirmation pop because we have another one.
Bug 20778: Fix subscription deletion. Also remove the first confirmation pop because we have another one. Bug 20778: (bug 19777 follow-up) Fix subscription deletion Bug 20778: (bug 19777 follow-up) Fix subscription deletion Bug 20778: (bug 19777 follow-up) Fix subscription deletion |
||
|
Description
Alex Arnaud
2018-05-18 12:37:25 UTC
Created attachment 75429 [details] [review] Bug 20778: Fix subscription deletion. Also remove the first confirmation pop because we have another one. Test plan: - go to a subscription detail page, - click on edit -> delete subscription, - confirm deletion, - check that the subscription is no longer here Patch tested with a sandbox, by Séverine QUEUNE <severine.queune@bulac.fr> Created attachment 75505 [details] [review] Bug 20778: Fix subscription deletion. Also remove the first confirmation pop because we have another one. Test plan: - go to a subscription detail page, - click on edit -> delete subscription, - confirm deletion, - check that the subscription is no longer here Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> I never noticed that point before :) I think we are not adding consistency in this script by removing a confirm from one of the js functions in a row of similar ones. Should you remove the other confirmation perhaps ? If you have confirm_deletion as name, I would expect a confirm there too. (In reply to Marcel de Rooy from comment #5) > I think we are not adding consistency in this script by removing a confirm > from one of the js functions in a row of similar ones. > Should you remove the other confirmation perhaps ? > > If you have confirm_deletion as name, I would expect a confirm there too. Hello Marcel, Not sure to understand what you are expecting. If i remove the 2 others JS confirmation (for closing and reopening), we have no more confirmation at all. I did it for deleting because there is an other one after. So the JS one is useless. Other possibility is to remove the one after JS for subscription deletion. I agree that this BZ is for fixing subscription deleting. Not for removing a useless confirmation. So i can just do what it is expected for (In reply to Alex Arnaud from comment #6) > Not sure to understand what you are expecting. If i remove the 2 others JS > confirmation (for closing and reopening), we have no more confirmation at > all. I did it for deleting because there is an other one after. So the JS > one is useless. > > Other possibility is to remove the one after JS for subscription deletion. > > I agree that this BZ is for fixing subscription deleting. Not for removing a > useless confirmation. So i can just do what it is expected for Yeah. Consider removing the one after the JS confirm. Caused by commit 666a1b276097785c2a7c33e718f26f5512b3d972 Bug 19777: Move template JavaScript to the footer: Serials, part 3 TT var used in .js Created attachment 75777 [details] [review] Bug 20778: (bug 19777 follow-up) Fix subscription deletion Caused by commit 666a1b276097785c2a7c33e718f26f5512b3d972 Bug 19777: Move template JavaScript to the footer: Serials, part 3 TT vars should not be used in .js files Test plan: Delete a subscription, confirm you see the confirmation popup and that the it deletes the subscription correctly after it has been confirmed. Created attachment 75778 [details] [review] Bug 20778: (bug 19777 follow-up) Fix subscription deletion Caused by commit 666a1b276097785c2a7c33e718f26f5512b3d972 Bug 19777: Move template JavaScript to the footer: Serials, part 3 TT vars should not be used in .js files Test plan: Delete a subscription, confirm you see the confirmation popup and that the it deletes the subscription correctly after it has been confirmed. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 75793 [details] [review] Bug 20778: (bug 19777 follow-up) Fix subscription deletion Caused by commit 666a1b276097785c2a7c33e718f26f5512b3d972 Bug 19777: Move template JavaScript to the footer: Serials, part 3 TT vars should not be used in .js files Test plan: Delete a subscription, confirm you see the confirmation popup and that the it deletes the subscription correctly after it has been confirmed. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Pushed to master for 18.11, awesome work all! This patch has been pushed to 18.05.x and will be in 18.05.01 Depends on Bug 19777 not in 17.11.x |