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

--- Comment #7 from Victor Grousset/tuxayo <[email protected]> ---
(In reply to Adolfo Rodríguez Taboada from comment #5)
> In Bug 38492 the function addItem was changed to replace _() with __() to
> make the string translatable as _() doesn't work. I have resubmitted the bug
> doing the same in addMulti

Good find. So that's how it was changed between your patch submission and my
review.

-------------

> As for the alert(), it is also used in cataloging_additem.js to check
> mandatory fields. I think it would be better to make or find a bug to change
> how Koha warns the user that there are mandatory fields empty.

The alert itself isn't a blocking issue:

(In reply to Victor Grousset/tuxayo from comment #2)
> At first I though adding an alert() would be a problem, since we are phasing
> them out since years:
> https://wiki.koha-community.org/wiki/Interface_patterns#Errors_and_messages
> 
> But here it's a bit different. There is already an alert() just above in
> addItem(). So the actual issue is the [almost] perfect duplication of code.

So there is still that duplication and now it's perfect with the switch to
__().
And we even had an example here of how duplicating code lead to a change in
only one of the duplicate instances.
If the common code was extracted to a subroutine, then when I applied the patch
it would have conflicted with Bug 38492 having landed in the meantime. And I
would have seen the difference and applied the i18n change.

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Reply via email to