Bug 18437 - Acquisitions fund edit repair
Summary: Acquisitions fund edit repair
Status: CLOSED INVALID
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Christopher Brannon
QA Contact: Testopia
URL:
Keywords: Academy
Depends on:
Blocks:
 
Reported: 2017-04-14 22:45 UTC by Christopher Brannon
Modified: 2019-10-14 19:57 UTC (History)
6 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
Bug 18437: Repair links & testing / Add buttons (1.66 KB, patch)
2017-04-14 23:06 UTC, Christopher Brannon
Details | Diff | Splinter Review
Bug 18437: Repair links & testing / Add buttons (REBASED) (1.66 KB, patch)
2017-04-18 23:21 UTC, Christopher Brannon
Details | Diff | Splinter Review
[BUG 18437] Fix selects / change links to buttons (1.99 KB, patch)
2017-04-21 15:35 UTC, Christopher Brannon
Details | Diff | Splinter Review
[BUG 18437] Fix selects / change links to buttons (6.33 KB, patch)
2017-04-21 15:58 UTC, Christopher Brannon
Details | Diff | Splinter Review
[BUG 18437] Fix selects / change links to buttons (6.66 KB, patch)
2017-04-21 19:45 UTC, Christopher Brannon
Details | Diff | Splinter Review
[BUG 18437] Fix selects / change links to buttons (6.83 KB, patch)
2017-04-21 20:15 UTC, Christopher Brannon
Details | Diff | Splinter Review
Bug 18437 - Acquisitions fund edit repair (6.92 KB, patch)
2017-09-01 17:40 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Brannon 2017-04-14 22:45:23 UTC
While editing a fund, if there is no owner and you select an owner, the owner does not appear until you save the edit and edit it again.

Also, when adding users, the selection tool is not checking if the user selected is already added.

Links next to owner and users should be converted to buttons.
Comment 1 Christopher Brannon 2017-04-14 23:06:54 UTC
Created attachment 62185 [details] [review]
Bug 18437: Repair links & testing / Add buttons

To Test:
1) Create fund.
2) Select owner.
3) Select user.
4) Select same user again.
5) Note owner does not show unless you save the changes and edit the
fund again.
6) Note user can be added multiple times.
7) Apply patch.
8) Repeat steps 1-4.
9) Note owner now shows first time.
10) Note user cannot be added multiple times.
11) Note links by owner and users have been changed to buttons.
Comment 2 Marc Véron 2017-04-16 16:40:36 UTC
Wanted to test but patch does not apply on current master:
fatal: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt).
Comment 3 Christopher Brannon 2017-04-18 23:21:11 UTC
Created attachment 62341 [details] [review]
Bug 18437: Repair links & testing / Add buttons (REBASED)

To Test:
1) Create fund.
2) Select owner.
3) Select user.
4) Select same user again.
5) Note owner does not show unless you save the changes and edit the
fund again.
6) Note user can be added multiple times.
7) Apply patch.
8) Repeat steps 1-4.
9) Note owner now shows first time.
10) Note user cannot be added multiple times.
11) Note links by owner and users have been changed to buttons.
Comment 4 Marc Véron 2017-04-19 13:51:18 UTC
Sorry, I can still not apply:

Wende an: Bug 18437: Repair links & testing / Add buttons
fatal: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt).
Dem Repository fehlen notwendige Blobs um auf einen 3-Wege-Merge zurückzufallen.
Kann nicht zu 3-Wege-Merge zurückfallen.

(Necessary blobs missing, can not fall back to 3-way-merge)
Comment 5 Christopher Brannon 2017-04-21 15:35:43 UTC
Created attachment 62534 [details] [review]
[BUG 18437] Fix selects / change links to buttons

To Test:
1) Create fund.
2) Select owner.
3) Select user.
4) Select same user again.
5) Note owner does not show unless you save the changes and edit the
fund again.
6) Note user can be added multiple times.
7) Apply patch.
8) Repeat stepps 1-4.
9) Note owner now shows first time.
10) Note user cannot be added multiple times.
11) Note links by owner and users have been changed to buttons.
12) Note "No owner" and "No users" messages have been added when there
are none.
Comment 6 Christopher Brannon 2017-04-21 15:58:29 UTC
Created attachment 62536 [details] [review]
[BUG 18437] Fix selects / change links to buttons

To Test:
1) Create fund.
2) Select owner.
3) Select user.
4) Select same user again.
5) Note owner does not show unless you save the changes and edit the
fund again.
6) Note user can be added multiple times.
7) Apply patch.
8) Repeat stepps 1-4.
9) Note owner now shows first time.
10) Note user cannot be added multiple times.
11) Note links by owner and users have been changed to buttons.
12) Note "No owner" and "No users" messages have been added when there
are none.
Comment 7 Christopher Brannon 2017-04-21 16:00:12 UTC
Okay, I think I got it this time.  Still figuring out all the quirks with git, so thank you for your patience!
Comment 8 Marc Véron 2017-04-21 17:33:59 UTC
It works as described, but while testing I found two issues:

Owners:
- If no owner is set, I see next 'Owner:' the text 'No owner', the 'Select' button and the 'Remove' butten. Remove makes no sense here. 
- Remove button has no waste bin icon (but for users it has).

Users:
- If I add a new user I can not remove again. (After submitting the page, removing  works). 


Not related to this bug, but for the records:
- On the select screen, th language selector hides the results 
  (especially on narrow screens). Is a language selector necessary here?
- On the select screen, if I accidentally click on the patron's name, 
  their detail screen is displayed in the pop up. No way to get back to 
  the selection screen. (You have to close the pop up and start over adding.)
Comment 9 Christopher Brannon 2017-04-21 19:45:36 UTC
Created attachment 62560 [details] [review]
[BUG 18437] Fix selects / change links to buttons

To Test:
1) Create fund.
2) Select owner.
3) Select user.
4) Select same user again.
5) Note owner does not show unless you save the changes and edit the
fund again.
6) Note user can be added multiple times.
7) Apply patch.
8) Repeat stepps 1-4.
9) Note owner now shows first time.
10) Note user cannot be added multiple times.
11) Note links by owner and users have been changed to buttons.
12) Note "No owner" and "No users" messages have been added when there
are none.
Comment 10 Christopher Brannon 2017-04-21 19:48:18 UTC
(In reply to Marc Véron from comment #8)
> It works as described, but while testing I found two issues:
> 
> Owners:
> - If no owner is set, I see next 'Owner:' the text 'No owner', the 'Select'
> button and the 'Remove' butten. Remove makes no sense here. 
> - Remove button has no waste bin icon (but for users it has).
> 
> Users:
> - If I add a new user I can not remove again. (After submitting the page,
> removing  works). 
> 
> 
> Not related to this bug, but for the records:
> - On the select screen, th language selector hides the results 
>   (especially on narrow screens). Is a language selector necessary here?
> - On the select screen, if I accidentally click on the patron's name, 
>   their detail screen is displayed in the pop up. No way to get back to 
>   the selection screen. (You have to close the pop up and start over adding.)

Removed remove button when there is no owner (NEW FUNCTION).  I was having trouble with this before, and it was not one of the issues I had originally focused on.

Fixed the problem with removing a user you just added.

Other issues, yes, I agree about the other issues.  Needs to be in a new bug.
Comment 11 Marc Véron 2017-04-21 19:51:56 UTC
(In reply to Christopher Brannon from comment #10)
> (In reply to Marc Véron from comment #8)
(...)
> > Not related to this bug, but for the records:
> > - On the select screen, th language selector hides the results 
> >   (especially on narrow screens). Is a language selector necessary here?
(...)
> 
> Other issues, yes, I agree about the other issues.  Needs to be in a new bug.

For the language menu see Bug 13835 with status NSO :-)
Comment 12 Marc Véron 2017-04-21 20:02:59 UTC
Works great now.

But sorry, QA tools complain:

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt
   FAIL	  tt_valid
		lines 528

I think it is related to:
https://wiki.koha-community.org/wiki/Coding_Guidelines#HTML1:_Template_Toolkit_markup_inside_HTML
Comment 13 Christopher Brannon 2017-04-21 20:05:33 UTC
Ugh!  I was contemplating that, but this was so much more elegant.  :/  Okay. I'll have it fixed in a moment.
Comment 14 Christopher Brannon 2017-04-21 20:15:51 UTC
Created attachment 62561 [details] [review]
[BUG 18437] Fix selects / change links to buttons

To Test:
1) Create fund.
2) Select owner.
3) Select user.
4) Select same user again.
5) Note owner does not show unless you save the changes and edit the
fund again.
6) Note user can be added multiple times.
7) Apply patch.
8) Repeat stepps 1-4.
9) Note owner now shows first time.
10) Note user cannot be added multiple times.
11) Note links by owner and users have been changed to buttons.
12) Note "No owner" and "No users" messages have been added when there
are none.
Comment 15 Christopher Brannon 2017-04-21 20:16:57 UTC
(In reply to Marc Véron from comment #12)
> Works great now.
> 
> But sorry, QA tools complain:
> 
>  FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt
>    FAIL	  tt_valid
> 		lines 528
> 
> I think it is related to:
> https://wiki.koha-community.org/wiki/Coding_Guidelines#HTML1:
> _Template_Toolkit_markup_inside_HTML

Okay.  Give it a whirl again.  (Crossing fingers)
Comment 16 Marc Véron 2017-04-21 20:39:44 UTC
(In reply to Christopher Brannon from comment #10)
> (In reply to Marc Véron from comment #8)
(...)
> > Not related to this bug, but for the records:
> > - On the select screen, th language selector hides the results 
> >   (especially on narrow screens). Is a language selector necessary here?
> > - On the select screen, if I accidentally click on the patron's name, 
> >   their detail screen is displayed in the pop up. No way to get back to 
> >   the selection screen. (You have to close the pop up and start over adding.)
(...)
> Other issues, yes, I agree about the other issues.  Needs to be in a new bug.

See Bug 18480 (NSO)
Comment 17 Lauren 2017-06-23 03:14:31 UTC
We tested this bug at the KohaCon 2017 and it works however we were not able to sign off.  We got this message "It seems you don't have applied a patch, so you cannot sign it off. If you applied patches from the right report, check the commit message of the last patch. It should start with "Bug XXXXX", if not, please inform the author of the patch."

Lauren & Maricon
Comment 18 Kyle M Hall 2017-09-01 17:40:00 UTC
Created attachment 66772 [details] [review]
Bug 18437 - Acquisitions fund edit repair

To Test:
1) Create fund.
2) Select owner.
3) Select user.
4) Select same user again.
5) Note owner does not show unless you save the changes and edit the
fund again.
6) Note user can be added multiple times.
7) Apply patch.
8) Repeat stepps 1-4.
9) Note owner now shows first time.
10) Note user cannot be added multiple times.
11) Note links by owner and users have been changed to buttons.
12) Note "No owner" and "No users" messages have been added when there
are none.

https://bugs.koha-community.org/show_bug.cgi?id=18437

Signed-off-by: Lauren Denny <ldenny76@gmail.com>
Comment 19 Kyle M Hall 2017-09-01 17:40:29 UTC
"No owner" isn't translatable in the javascript.
Comment 20 Katrin Fischer 2017-09-05 21:17:39 UTC
Thx for catching the translation issue Kyle. There is another here:
$("#budget_users").prepend('<li id="user_none">No users</li>');
Comment 21 Christopher Brannon 2018-09-16 16:47:07 UTC
Reworking - owner shows as null/null when added, and doesn't show at all after saving and editing fund again.
Comment 22 Christopher Brannon 2018-09-16 17:00:25 UTC
Shows null, null if no names in account.  This is very insignificant.  Everything else of concern has already been addressed.  Abandoning this bug.