OpenILS::Utils::BadContact->mark_users_contact_invalid ignores additional note argument

[Josh Stompro]

EG 3.0

OpenILS::Utils::BadContact->mark_users_contact_invalid accepts an argument for an additional note but it isn't actually used anywhere. I think the additional note should just be added on to the end of the default note, which is set to the value of the contact info that is being marked invalid.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=blob;f=Open-ILS/src/perlmods/lib/OpenILS/Utils/BadContact.pm;hb=HEAD

I'm working on a script that scans an imap folder and automatically deals with certain bounces by invalidating contact info, and I would like to be able to add the bounce reason to the penalty note.

Josh

[Josh Stompro (u-launchpad-stompro-org)]

Working branch at user/stompro/lp1752334_badcontact_use_additional_note

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

Josh

[Jane Sandberg (sandbergja)]

I just eyeballed this, and it looks good from my initial observation. It also looks like it would be a really good candidate for a live test, perhaps using this live test as an example: https://github.com/evergreen-library-system/Evergreen/blob/master/Open-ILS/src/perlmods/live_t/zz-lp1779467-mark-item-discard.t

The live test could perhaps call open-ils.actor.invalidate.email with the note field, and then retrieve the ausp object and verify that the note is appended correctly.

Thoughts?

[Josh Stompro (u-launchpad-stompro-org)]

Jane, I'll give creating a live test a try.

https://wiki.evergreen-ils.org/doku.php?id=dev:contributing:qa#perl_live_tests

[Josh Stompro (u-launchpad-stompro-org)]

Here is a new working branch that applies cleanly to master.

user/stompro/lp1752334-badcontact-add-note-v2

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/stompro/lp1752334-badcontact-add-note-v2

I've included a live test that tests out the various options associated with open-ils.actor.invalidate.*, including the new behavior of being able to add an additional note.

Josh

[Jane Sandberg (sandbergja)]

Nice test! Thanks, Josh. This works for me. Signoff at user/sandbergja/lp1752334-badcontact-add-note-v2-signoff

[Galen Charlton (gmc)]

Since the methods in question already documented the additional notes parameter, I decided to classify this as a bugfix rather than an wishlist.

Consequently, I've pushed this down to rel_3_9, along with a follow-up to clean up how it checks whether a note has been supplied or note. Thanks, Josh and Jane!