Pkp-lib: Add filter by last modified more than X days ago to submissions lists

[asmecher]

• [x] Clean up data model (for @asmecher)
• [x] Add fitering to UI (for @NateWr)

[asmecher]

PRs:

• https://github.com/pkp/pkp-lib/pull/4169
• https://github.com/pkp/ojs/pull/2144
• https://github.com/pkp/omp/pull/600

These standardize the last_modified and date_status_modified columns so that last_modified gets bumped on every UPDATE submissions, and date_status_modified gets bumped whenever something important happens (= a logged event). I'd suggest relying on the latter.

[NateWr]

:+1: should I pull your branches in and build on top of that? Or do you want to merge to master and I'll implement the filtering based off that?

[asmecher]

@NateWr, I've merged those branches into master, thanks!

[NateWr]

The work that's left on this is pretty small and it's a huge priority every time we do the UI/UX groups at sprints. It feels like we should really make progress on this. @asmecher would you mind if I add this to the 3.2 milestone?

cc @alex-wreschnig

[asmecher]

@asmecher would you mind if I add this to the 3.2 milestone?

OK by me!

[NateWr]

@forgive38 If you look above, Alec has already done the backend work we need for a filter that would return submissions that have not been modified in the last X days.

For 3.2, we'd like to add a simple filter that returns submissions inactive for the last 30 days.

If you could do that, it'd be great!

[forgive38]

@NateWr : I looked at lastModified and dateStatusModified but I found sometimes these dates are not updated.
For example, when a reviewer complete a review these dates are not updated (even if you - as an Editor - confirm review and thank reviewer)
while when reviewer accept to review, these dates are updated

also no event when review are in, only these events in history :

• An email has been sent: [] Able to Review
• reviewer has accepted the round 1 review for submission 17.

So I think, if we want to have an analytic view for a journal editor, we have to rely on dateStatusModified and so, this one has to be updated on each modification

what do you think about it?

[NateWr]

It's been a while since I was looking at this issue, but I think what you need to look at is the date of the latest entry in the editorial history of a submission. Every piece of activity on a submission should be recorded here, and if it isn't, it should be added.

[forgive38]

First, proposition to add date Status Modified in submission list
PRs:
pkp/ui-library#34

5016

pkp/ojs#2450

[forgive38]

@NateWr : I'm thinking about "Inactive for 30 days" : the relevant time depends on journal, because in our case, sometimes delay between review and acceptation is around 15 days and for another journal it's around 3 or 6 months: so I think order by date is more useful/
or number of days is a journal settings ?

[forgive38]

update PRs with isInactive filter.

[NateWr]

@asmecher I notice that we're updating the dateStatusModified whenever a submission log entry is recorded. But submissions also have a lastModified column, and this gets updated during the insertObject and updateObject DAO methods.

Am I right in thinking that lastModified only tracks when the submission object itself is updated and dateStatusModified tracks the last activity related to the submission?

[NateWr]

@forgive38 I've left some comments on the PR, particularly about the set number of inactive days for the filter: https://github.com/pkp/pkp-lib/pull/5016#pullrequestreview-279480478

[asmecher]

Am I right in thinking that lastModified only tracks when the submission object itself is updated and dateStatusModified tracks the last activity related to the submission?

The "last modified" etc. timestamps are not very widely used and we don't have a single policy about whether they should be updated e.g. when some object belonging to a submission changes. IMO a policy would need to be developed considering how the data is going to be used -- e.g. if we need fine-grained information about what aspect of a submission changed most recently, then cascading an update up to the submission level would clobber useful information; however, if we just need a quick way to sort submissions by activity, then a cascading update makes sense.

[NateWr]

I can imagine four kinds of modification dates to track for a submission:

1. lastModified when the submission object itself was last changed (ie - insert/update methods in the DAO). It doesn't get updated when related objects are changed.
2. dateStatusModified when the submission's status was changed (ie - from STATUS_QUEUED to STATUS_PUBLISHED).
3. dateStageModified when the submission's current stage was changed.
4. dateLastActivity when any related object is changed (cascading update)

I'm not sure if we have real use cases for 2 and 3 right now, and those dates will be available in the activity log.

We have 1 and (mostly) 4, except that 4 is currently named dateStatusModified. Maybe we should rename it to dateLastActivity?

We don't need to do it as part of this issue but it could be a change we aim to make down the line.

[asmecher]

Sounds fine, thanks, and I agree that we shouldn't make this change yet -- until we're using the data it'll be prone to unreliability/decay.

[NateWr]

@forgive38 Thanks very much! I've merged your work on the filter into our stable branch. We'll still need to port this work over for master but that should be pretty easy.

Would you be willing to do one more bit of work? I know I asked you to remove the date modified from the submission summary, but would you be able to add it back in to the details table that appears when you open a submission up?

We'll go with this for the stable version and try to get it into the summary for 3.2 or shortly after.

[forgive38]

@NateWr : I add some commits to insert dateStatusModified in submissions list

[NateWr]

Thanks @forgive38. These commits look good. A couple of things:

First, right now you're concatenating the string and the date manually. But this needs to be done inside the translated strings so that other languages can reorder things as needed. So this:

<message key="common.lastActivity">Last activity recorded on</message>

Should be:

<message key="common.lastActivity">Last activity recorded on {$date}.</message>

Then in the template we can do this:

{{ __('lastActivity', {date: item.dateStatusModified | formatDate} }}

Second, in the filter, let's return an empty string if no date is passed, rather than the (n/a):

if (!str) return '';

[forgive38]

@NateWr : I updated PRs

[NateWr]

Thanks!

PRs:
https://github.com/pkp/pkp-lib/pull/5092
https://github.com/pkp/ui-library/pull/37

[NateWr]

Thanks @forgive38! I've merged the PRs to the stable branch. We'll still need to do the same for the master branch for 3.2, so I'll leave this open.

[forgive38]

I will try to port that to the master branch before I try to make same thing for #4103 because I saw many things have changed in ui-library.

[forgive38]

Hi

PR for master :
~pkp/ojs#2495

5144

pkp/ui-library#42~

(update locale in new format)

Just a question about https://github.com/forgive38/pkp-lib/blob/72d8b362f556ba223639d9ce64e7418db56a0509/composer.json#L37

we can't use the "original" adodb ?

Thank you

[NateWr]

This looks great, thanks! I will do a more hands-on review in the next few days and get it merged.

we can't use the "original" adodb ?

Good question! I'm not up-to-speed on all of the details here, but I know that Alec has a plan for deprecating our use of ADODB entirely: https://github.com/pkp/pkp-lib/issues/2493.

[NateWr]

This is great, @forgive38! I've worked on top of your commits to migrate from date_status_modified to date_last_activity and made a couple more minor changes.

New PRs:
https://github.com/pkp/pkp-lib/pull/5155
https://github.com/pkp/ui-library/pull/44
https://github.com/pkp/ojs/pull/2498
https://github.com/pkp/omp/pull/716

It looks like I didn't stage the OJS PR correctly. I may not get a chance to fix and merge until the end of the week, but it will go in. Thanks! :+1:

[NateWr]

All merged. Thanks very much for your work on this @forgive38! Will we see you at the PKP sprint in Barcelona?

@amandastevens this _may_ effect documentation. It adds a new filter to the submissions list to view submissions that have had no activity for 30 days or more.