> Since installing this code on a 3.4.2 PINES test server broke things, we tried it again on a fresh server with current master and Concerto data. To test, we set up the clean server, then added some notes, messages, and alerts to accounts before applying the new code and conversion script. Again, the code broke a lot of existing functionality (eg, the only account that could log in to staff client was admin, no accounts could log into OPAC, account edits produce errors...)

Hrmm.  Staff logins are working for me, but for patrons, I do see this for both editing patrons and trying to login as them:

ERROR:  record "old" has no field "alert_message"

I see a missed alert_message field in the bowels of Storage, but that doesn't fix it.  Hrmm.  Okay, it's the functionality for preserving data in the auditor, which can get triggered during login when passwords are migrated to their new salted versions.  I'll give that a look.  Good catch!

> Problems Found with Old Converted Data:
> -> Conversion made a patron visible note *not* patron visible, and did not record staff person who'd entered it. The Staff Alert, Label, and Penalty Name are also blank.

I see the same.  I've fixed the visibility part.  Re: penalties and recording of the staff person, here's the situation: I kept all existing user messages in place (though neglected to flag them as public), and at the time only explicitly migrated private notes, with the thought that the public notes would be redundant (under the current scheme where the creation of a public patron note creates an associated user message).  User messages currently do not record which staff created them.

So what we could do is, at upgrade time, try to link public patron notes with their associated user messages, based on matching user, message content, and timestamps (there's no explicit link, alas, and timestamp matching may have to be fuzzy).  And then for public patron notes that we can't match (say, because the patron true deleted the associated user message), we can migrate the public patron note as archived, so that it doesn't show up for the patron, and so that staff still have a record of it.  How does that sound?  It doesn't match current behavior exactly, but current behavior is arguably non-ideal anyway (the split of notes into user messages).  Staff Alert and friends would still be unset; there's really no need to create a standing penalty for these and that's the intended behavior you get when there is only a user message with the combined penalty/user-message view.

> -> Converted notes have blank Org Depth - should they?

No org depth, as long as they're still visible to everyone who needs to see them.  Worse comes to worst, we could create SILENT_NOTE penalties to go with migrated notes that are not archived, but my preference is not to.

> -> Alert that was moved from the patron edit screen to this interface gets a creation date of the date the script processed it. I think this is probably the way it should work, but staff will need to understand that those alerts are actually older than the date stamp. Maybe when those alerts are converted, a message could be appended to them saying "(Converted to new format during upgrade)" or something like that?

Or we could set the title to that effect as well.  I'm in favor.

> I then created some new notes of various types and visibility:
> -> It would be helpful if double-clicking on a note would open in Edit (right now it does nothing, you have to use the Actions dropdown or right-click to open)

Agreed.  I might punt that to a separate enhancement ticket after this goes in.

> -> If a non-alerting note is present, it no longer indicates in the patron summary bar that there is a note - without that indicator, staff will have no idea there are notes present.

It should get represented in the count next to the Notes button.  That should be prominent enough for a non-alert note, yeah?

> -> When creating a new note/alert/block, it defaults to the branch level. PINES needs this to default to the consortium level (especially for blocks). Maybe a new setting is needed?

You can change the default depth for those standing penalties explicitly to 0 and it should work.  I believe those depths serve no other function (for staff-generated penalties) now that you can set the context org/depth explictly per message.

Thanks Terran!  I'll push a new rebased branch after I have all these fixes assembled.