Duplicate Code Entries in ils_events.xml

[Jason Stephenson]

Evergreen: master as of 2014-09-04
Also affects 2.6.

I have noticed that there are two code numbers duplicated in ils_events.xml.

7025 is used for PATRON_TOO_MANY_ACTIVE_PASSWORD_RESET_REQUESTS and COPY_STATUS_LONG_OVERDUE.

7026 is used for PATRON_NOT_AN_ACTIVE_PASSWORD_RESET_REQUEST and COPY_STATUS_LOST_AND_PAID.

It looks like this may have happened because the earlier entries for password reset are "out of order" with the rest of the number sequences.

It's not causing any problems but something we might want to fix.

[Michele Morgan (mmorgan)]

I noticed this as well, confirming.

[Chris Sharp (chrissharp123)]

Well, looks like this bug *is* causing a problem in PINES that was just detected recently. When staff click on "Send Password Reset Link" in the Patron Registration/Edit UI, the message "Copy is marked as long-overdue" pops up, causing understandable confusion on the part of staff.

[Chris Sharp (chrissharp123)]

Branch to fix the duplicated numbers here:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/csharp/lp1369345_duplicate_ils_event_codes

I also decided to clear up whitespace inconsistencies (tabs vs. spaces, spaces at ends of lines).

Tested on PINES production.

[Jason Stephenson (jstephenson)]

I am not able to reproduce the issue reported by Chris Sharp in comment #2 on a test system running
 Evergreen rel_3_10. I'm also not sure how changing the ils_events.xml would resolve that issue since it doesn't appear to come into play in the AngularJS interface. Am I looking in the wrong place? Is there an older, Dojo interface still hanging around that I should be looking at?

I also have noticed that the Angular patron registration/edit interface seems to lack the button to send a password reset email request. That is a different bug, however.

That said, Chris's changes look sane to me and didn't seem to cause any issues when I applied them to that same rel_3_10 test server, so I have pushed a signoff branch:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/dyrcona/lp1369345_duplicate_ils_event_codes-signoff

A final note about the patch itself: it would be easier to review if the general whitespace cleanup was separated into its own commit apart from the functional changes. I'm not asking for a repatch because I was able to suss out the differences. I know some other core developers would ask for such a change, though, and I probably would if the patch were more complicated.

[Galen Charlton (gmc)]

I can see how the mismatch between the event and the displayed description happens: open-ils.actor.patron.password_reset.request can throw the PATRON_TOO_MANY_ACTIVE_PASSWORD_RESET_REQUESTS event, including the localized description, which AngularJS in turn will display. However, OpenILS::Event loads ils_events.xml, it looks up the descriptions by the event's numeric code, not the event's text code, so duplication of the numeric code can overwrite the correct descriptions.

I share Jason's preference that significant whitespace cleanup be separated from patches that make substantive changes, but since "git show -b" could manage, so be it. Tested and pushed down to rel_3_9.

Thanks, Chris and Jason!