Clean up the submission form: one metadata store, availability statement, config versions - #165
Merged
Merged
Conversation
…tings # Conflicts: # CHANGELOG.md # classes/Settings/SettingsForm.php # locale/en/locale.po # templates/settings.tpl
nuest
added a commit
that referenced
this pull request
Aug 26, 2026
…n, and the API handler (#166) Three commits that were pushed straight to `main` and one new one, brought together as a reviewable change. `main` is being reset to the merge of #165 so all of it lands here instead. ## What this covers Four parts of the plugin had no test at all. Writing the tests turned up three production bugs, two of them silent. ### Issue TOC badge and the status API (8ff1fd1) - `IssueTOCUnitTest` for the setting and opt-in gates, and `issue-toc-badge.cy.js` for what the badge actually renders — image variants, height, the text-only form with its configured wording and colour, and where it links. Each variant is also captured to a screenshot, so the settings can be looked at rather than inferred. - `status-handler.cy.js` for the status API end to end: pending until recorded, newest record wins, append-only history newest first, rejected payloads, and the automatic update that stops deciding once a person has. - **New setting:** the badge can link either to the certificate's page in the register or to its DOI, for both the issue TOC and the article page. Whichever the journal did not pick stands in when the preferred one is missing. - **Bug found:** an existing test pinned a certificate format the plugin never produces (`CODECHECK-2025-001`), which hid that every badge linked to `href=""`. ### Publication validation (f400f4d) **Bug found: the CODECHECK gate on publishing had never worked.** The validator asked the router's handler for the authorized submission, but publishing goes through the REST API, where there is no handler. Every publish attempt threw inside the hook, PKP logged "failed to handle the hook" and published anyway — so the status allow-list, the YAML check and the extended repository check were all inert. The submission now comes from the hook arguments. Covered by unit tests for the opt-in gate and the no-handler case, plus `publication-validation.cy.js` driving the endpoint OJS publishes with — including one test that unpublishes a ready article, watches CODECHECK refuse it, then accepts the status and publishes it for real. ### API handler testability (65f45d9) `CodecheckApiHandler`'s constructor resolved the route, authorized and served, and serving ends in `exit`, so nothing could build one without it answering a request and killing the process. The request cycle is now `execute()`, and responses go through a `JsonResponseEmitter` instead of a static call that echoes and exits. `emit()` is declared `never`, because every endpoint body is written assuming that responding ends the request; the test emitter throws where production exits. That makes the plugin's only access control testable — it sits outside PKP's authorization policies and checks the CSRF token and roles itself, in front of nineteen endpoints, several of which write to the public register. The token is now checked before the route is resolved, so an unauthorized caller cannot probe for routes. **Bug found:** `setupAPIHandler()` called `$router->setHandler($apiHandler)`. `setHandler()` takes a `PKPHandler` and this is not one — the call was unreachable only because the constructor exited first. Serving from `execute()` made it live, it raised a TypeError, PKP swallowed it, and OJS answered every plugin API call with its own 404. Caught by the e2e suite, not by PHPUnit. Also: a POST to reserve an identifier or update the register issue that omits a required field now answers 400 naming the field, instead of raising "Undefined array key" on its way to a 500. ## Tests PHPUnit 139 → 201, Cypress component 61 (unchanged), e2e 19 across 5 specs → 40 across 9. All green locally against OJS 3.5.0-5.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #152.
The author's CODECHECK entries and the codechecker's were kept in two unsynchronised stores, and the submission wizard fed the one nothing reads. This branch settles that and finishes the surrounding work.
One store. The wizard writes into
codecheck_metadatadirectly. The publication-levelcodeRepository,dataRepositoryandmanifestFilesfields are removed, so there is nothing to merge or reconcile. The author's entries arrive in the codechecker's editable repository and manifest lists marked as author-provided: editable, so a typo can be fixed, but not deletable. Saving is a merge — an entry the author still lists keeps whatever the codechecker did to it. TheisPrivateflag is renamedhidden, one word for one idea.Data and software availability statement on the article page. Shown in the main column below the abstract, via
Templates::Article::Main. An article whose author provided no statement says so rather than staying silent — an absent section cannot be told apart from a journal that never asked. Three settings: show the section, leave it out when empty, and rename its heading.Config versions. A journal chooses which CODECHECK config file specification versions codecheckers may pick, defaulting to 1.0 alone rather than every known version, so a check records the specification it was done against. The generated
codecheck.ymlnow declares the recorded version instead of always claiming 1.0, and the metadata form's introduction links to the specification for the selected version.Settings page consistency. The availability settings are grouped like the submission, GitHub and publication groups; the badge / logo group had sat outside
{fbvFormArea}and never got the bordered box; the statuses that permit publication are a plain checkbox list rather than a hover menu, removing the form's last dropdown; and "Clear / Reset DB" moved to the bottom. Journals showing text instead of a badge can set both the wording and its colour.Also fixed along the way:
getMetadata()readdataAvailabilityStatementoff the submission while everything else used the publication, so the API returned null; the workflow state still injected three fields that no longer exist; and the metadata form's read-only panel carried markup for them.Follow-up filed as #164: settings that replace a localised string are stored as one plain string, so a multilingual journal has to pick one language.
Testing: PHPUnit 139, Cypress component 61, e2e 16 — all green. The e2e settings round-trip covers the new settings automatically. The test dataset needed no change: no stored shape changed in this branch beyond what the earlier commits already migrated.