Sync the values of editorial decision constants across all applications

[NateWr]

Describe the problem you would like to solve
Editorial decision types are tracked by constants like Decision::ACCEPT. However, many of the same constants have different values in OJS and OMP. Examples:

Constant	OJS Value	OMP Value
Decision::ACCEPT	1	2
Decision::PENDING_REVISIONS	2	4

As a result, these constants must be defined separately in each application, resulting in duplicate code. It also makes it harder to interpret database records from one app or another.

Describe the solution you'd like

• Make the values of editorial decision constants the same across all applications.
• Add an upgrade migration that changes existing records to match the new values.
• Move all editorial decision constants into lib/pkp.
• Remove deprecated constants from OPS.

Additional information
This work depends on #7265.

[touhidurabir]

Move all editorial decision constants into lib/pkp.

@NateWr we have and will still have some CONSTANTS specific to OMP. you think it's a good idea to move those to lib/pkp when we have those some decisions related to those only in OMP ? I don't see a problem with it but we were trying not to out only some specific details in the shared lib unless it's used by multiple apps .

Also as I have checked, all the OJS constants are all are present in the OMP so making adjustment only for OJS will reduce the migration and sync up complexity quite a bit . However this leads me to few questions

1. There are some sequential gaps between two constants, for example SEND_TO_PRODUCTION = 7 and INITIAL_DECLINE = 9 and there is no constant set for 8 . I am guessing this is not intentional, right ? And better to make the constant value fully sequential, right ?
2. Bit confused about all these global constants SUBMISSION_EDITOR_DECISION_INTERNAL_REVIEW as found no use of those . any where they have been used that I am missing ?

[touhidurabir]

@NateWr please review

pkp-lib --> pkp/pkp-lib#7725 Decision constants sync up
ojs --> pkp/pkp-lib#7725 Decision constants sync up
omp --> pkp/pkp-lib#7725 Decision constants sync up
ops --> pkp/pkp-lib#7725 Decision constants sync up
orcidProfile --> pkp/pkp-lib#7725 decision class path update

[NateWr]

Sorry for the delay in replying to your previous questions, @touhidurabir. I've reviewed the code now.

we were trying not to out only some specific details in the shared lib unless it's used by multiple apps

Yes, and Alec might not like me recommending this. But I think it's much easier to have all of the constants defined here in one place. It will prevent us from accidentally getting them out of sync again.

better to make the constant value fully sequential, right ?

I like how you've done it. I'm sure that was a mind-numbing bit of work. 👍

Bit confused about all these global constants SUBMISSION_EDITOR_DECISION_INTERNAL_REVIEW as found no use of those . any where they have been used that I am missing ?

This is the old constant for Decision::INTERNAL_REVIEW. So any code for 3.3 or below will use SUBMISSION_EDITOR_DECISION_INTERNAL_REVIEW. We keep these constants around, for now, to ease the migration of third-party code, which may rely on these constants.

[asmecher]

Alec might not like me recommending this

Not enough to bother mentioning it; there are benefits to both approaches. We might actually move to enums later, and those would require us to specify them all in pkp-lib; it'll be impossible to extend them in the apps. So no objections here.

[touhidurabir]

@NateWr updated PR

pkp-lib --> #8218
ojs --> pkp/ojs#3520
omp --> pkp/omp#1188
ops --> pkp/ops#339

[NateWr]

All merged to main. Thanks @touhidurabir!

[asmecher]

@touhidurabir, could you have a look at the Travis builds? This appears to have broken the tests for all 3 apps.

[touhidurabir]

@asmecher I looked into the Travis build. it passed for OJS and OPS. The build for OMP was canceled after a failed build and we discover that that was due to a typo into the upgrade.xml so I guess once that fixed, we canceled the test for OMP and merged it . can you provide the details where it's breaking or the travis log ?

[asmecher]

Sure, these are the 3 builds where i7725_main were merged:

• https://app.travis-ci.com/github/pkp/ojs/builds/254979579
• https://app.travis-ci.com/github/pkp/omp/builds/254979582
• https://app.travis-ci.com/github/pkp/ops/builds/254979584

The previous builds all appear to pass.

They all appear to fail on the application's API.spec.js. (Actually, I wonder if it's #7366 that caused the failures?)

[touhidurabir]

@asmecher the build failing is not related to this issue but related to #7366 . it's failing for the cypress/tests/integration/API.spec.js with following error

API tests
       Configures an author's API key:
     AssertionError: Timed out retrying: Expected to find element: `form[id="apiProfileForm"] button:contains("Create API Key")`, but never found it.
      at Context.eval (http://localhost/__cypress/tests?p=cypress/tests/integration/API.spec.js:27:8)

but at the same time passing for similar one lies at lib/pkp/cypress/tests/integration/API.spec.js is passing . So I think build failing is not related to the PR of this issue .

weirdly I can not generate the screenshots for the log file using the command uudecode /path/to/log.txt | tar xvz as getting error such as

uudecode: cypress-failed-log.txt: stdout: character out of range: [33-96]
tar: Error opening archive: truncated gzip input

@asmecher can you generate the screenshots ? also may be better to move this discussion to #7366 and reopen it .

[NateWr]

Yeah I merged this yesterday and can confirm all three app's tests were passing on the PR before merge.

[touhidurabir]

@NateWr yes , the problem is not related to this one but I think related to #7366 . I have filed as issue #8223 and made PR that has updated test (only for OJS now). Hope that fix the build failing issue . Weird part is when the PR made for #7366, we had no issue with build passing .

[asmecher]

OK, thanks to you both! Closing this again in favour of #8223.