Wishlist: Invoice Paid Timestamp

Bug #1635354 reported by Leslie St. John
14
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Wishlist
Unassigned

Bug Description

EG 2.9.1

There have been several times when a library needs to know what invoices have been paid during a certain time period. The only timestamp associated with invoices is creation. Currently there is no way, that we have discovered, to construct a search or report that can provide this data set. We would like a timestamp associated with closing an invoice. If the invoice is later reopened, changed and closed then a new timestamp would be recorded. If going all the way then a history of the invoice would also be available, similar to the PO History.

Revision history for this message
Bill Erickson (berick) wrote :

I propose was also add a "closed by" field, which tracks the Evergreen user the marked the invoice as closed.

Changed in evergreen:
status: New → Confirmed
assignee: nobody → Bill Erickson (berick)
Revision history for this message
Bill Erickson (berick) wrote :

Some implementation details / questions.

Should we drop the acq.invoice.complete column and replace any references with checks for non-NULL-ness on the new close_date field? Good for data consistency and simplifying code, not so great for having to rebuild reports. (Though, arguably, many such reports would be rebuilt in light of the new columns).

Related, we'd have to back-fill close_date and closed_by values for existing complete invoices during the upgrade. I would favor applying values regardless, but if we drop 'complete' it will be non-negotiable.

Is there precedent for back-filling a user id?

Revision history for this message
Tiffany Little (tslittle) wrote :

A question in return: If we did away with the acq.invoice.complete column, say that a user closes the invoice and close_date and closed_by values are generated. What if the user then re-opens the invoice and it remains open for whatever reason (potentially user error). Would a report looking for open invoices (presumably checking for a NULL value in the close_date?) find that invoice since it had a close_date at one point? If it wouldn't find that invoice, I could see keeping acq.invoice.complete since that's a yes/no if the invoice is open. (I think.)

Revision history for this message
Bill Erickson (berick) wrote :

Hi Tiffany, yes, the report would look for a NULL value in close_date. Like today, if an invoice was re-opened, it's considered open, and would be skipped by the report. An invoice with complete = true and no close_date would be confusing and probably not something we would want to allow in the code. Avoiding that scenario is why I suggested dropping the complete column.

Also like today, if you want to know whether an invoice was closed and reopened, you'd have to look at the audit tables. If tracking invoice reopening needs to be more prominent and accessible to the interface, though, we could potentially create a new table to track close/open dates for each invoice, since it can happen more than once.

Revision history for this message
Bill Erickson (berick) wrote :
Revision history for this message
Bill Erickson (berick) wrote :

Remaining code and release notes force-pushed to same branch.

From the release notes:

Acquisitions invoices have 2 new fields:

* Close Date -- This is set to the time when the ACQ user clicks the "Close" button in the invoice interface.
  ** This field 'replaces' the existing 'complete' field. An invoice is considered complete if a close date value is set.
* Closed By -- This is set to the logged in staff user who performs the "Close" action.

As with the now-defunct 'complete' field, these new fields are cleared in the event an invoice is reopened.

These new fields are visible in the invoice interface under the 'Show Details' action for closed invoices.

Upgrading Invoice Reports
+++++++++++++++++++++++++

Existing report templates that reference the invoice 'complete' field should be modified to check whether the new close_date field is NOT NULL instead.

Other Upgrade Considerations
++++++++++++++++++++++++++++

At deploy time, all invoices with a 'complete' value of TRUE will have their 'close_date' field set to NOW. A value is required, since this field is now the source of whether an invoice is open or closed.

However, no values will be applied to the closed_by field for already closed invoices.

--------

To test:
[1] Create an ACQ invoice.
[2] Close the invoice
[3] Confirm the "Show Details" action now displays the new Close Date and Closed By fields in the invoice summary along the top.
[4] Reopen the invoice and confirm the fields no longer display and that the values have been cleared in the DB.

Adding pullreq.

tags: added: pullrequest
Changed in evergreen:
milestone: none → 3.1-rc
milestone: 3.1-rc → 3.next
assignee: Bill Erickson (berick) → nobody
Dan Wells (dbw2)
summary: - Whishlist: Invoice Paid Timestamp
+ Wishlist: Invoice Paid Timestamp
Revision history for this message
Bill Erickson (berick) wrote :

I have rebased the branch to current master and pushed an additional fix to the "Open Invoices" interface to use the new 'close_date' field instead of the 'complete' field. This fix includes a general change to the ACQ unified search interface to allow for searching on NULL date values.

To test NULL date searching, open the ACQ unified search interface, select a date field to search, use the 'is' or 'is not' matcher, and leave the text input empty to signify NULL.

Bill Erickson (berick)
Changed in evergreen:
milestone: 3.next → 3.2-beta
Bill Erickson (berick)
Changed in evergreen:
importance: Undecided → Wishlist
Kathy Lussier (klussier)
Changed in evergreen:
assignee: nobody → Kathy Lussier (klussier)
Revision history for this message
Kathy Lussier (klussier) wrote :

I don't think this is a dealbreaker, but I would like to make an argument for keeping the closed column in acq.invoice. I do have a concern about broken reports, but I think the release notes entry you already wrote can help address that concern.

In addition to that concern, we also find that the invoice search UI is a little more intuitive with the presence of a closed field, where you could just check a box to indicate whether an invoice is closed or not. By relying on the invoice closed date field, it becomes a little clunkier for users where they have to change the 'is' field to 'is not' and then click to make the calendar picker disappear. I don't know that most people will immediately know that they will be able to search closed invoices in this manner.

See https://drive.google.com/file/d/1rhGb24CzgCJLwpINlUyETbtPTFiZChoQ/view

It's a small thing, and I would rather get the feature in sooner if there is disagreement or it just isn't doable in the time we have left before feature freeze. But I'm throwing the question out there just in case it can be done.

Thanks Bill!

Revision history for this message
Kathy Lussier (klussier) wrote :

Since my concerns were minimal, I went ahead and merged this (with 5 minutes to spare!) in the interest of getting this into 3.2.

Thanks Bill! I think our acq librarians will find this to be useful!

Changed in evergreen:
status: Confirmed → Fix Committed
assignee: Kathy Lussier (klussier) → nobody
Revision history for this message
Tiffany Little (tslittle) wrote :

One of our Acq libraries will be happy to see this in 3.2! As a discussion point, I do have to say that I second Kathy's thoughts about keeping the "closed" column. It definitely seems much more intuitive for users to simply check Closed?: Yes/No. Having the date for additional information is wonderful, but having to search by its absence seems a bit clunky.

Revision history for this message
Christine Morgan (cmorgan-z) wrote :

I think our libraries will find this enhancement to Acquisitions really helpful. Thanks so much Bill!
I do also agree with Kathy and Tiffany that keeping the "closed" column and using the check for yes or no is much more intuitive, and requires far fewer clicks, than searching for the absence of a closed date.

Revision history for this message
Bill Erickson (berick) wrote :

I can certainly understand not wanting to have the search be less intuitive. My hope is we can do that without adding the redundant column back, maybe with some UI magic. Should we open a new bug?

Revision history for this message
Kathy Lussier (klussier) wrote :

Sure. I can add it.

Thanks Bill!

Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.