[iudizm]

• Including grant award information from Funding plugin in the message to OAS.

[bozana]

Hi @iudizm,

Thanks for the update! We now review plugin releases with the help of Claude, and have found some issues that already existed in earlier releases:

Main points

• Publication date (3.0.x): P1Pio.php:103 reads datePublished from the submission, but in 3.5 it is stored on the publication. As a result, every message gives the publication date as 1970-01-01. Could you read it from the publication instead, e.g. $submission->getCurrentPublication()->getData('datePublished')? The test stub sets it on the submission, which hides the problem.
• DOIs created at publication: with core's "upon publication" DOI setting, the DOI only exists after the plugin has checked the requirements, so nothing is sent. It might be worth mentioning in the README that DOIs need to be assigned before publishing.
• Queue workers (3.0.x): when jobs run from the command line (jobs.php), generic plugins are loaded at startup without a journal. getEnabled() is then false, so register() doesn't add the Schema::get::submission hook. As a result, Repo::submission()->edit() drops the three status properties, and the panel stays at "queued". In PKP we register schema extensions (and other hooks that are needed outside a request) outside the getEnabled() check, and check whether the plugin is enabled inside the hook callback instead, taking the context from the object being handled. Could you do the same here?
• Repeated sends (3.0.x): Publication::publish also runs when a new version is published or an article is re-published, and recordPending() overwrites a "sent" status, so each publish sends another P1-PIO message. We assume this is intended, to pass on updated metadata. How does OA Switchboard handle a second message for the same article: as an update, or as a new notification to the recipients? It might be worth mentioning in the README.
• Fixed article type and OA policy: every message says research-article and pure OA journal, whatever the section or the journal's model. Could you mention this in the README?
• Missing api_key_secret line: APIKeyEncryption only checks for an empty string. If the line is missing, the value is null, which causes a TypeError and breaks installation. Could you treat null the same as ''?
• OASwitchboardManage default branch: it calls the plugin's manage() again, which keeps recursing until PHP runs out of memory. Could you return new JSONMessage(false) there?

Smaller points

• Both API routes are anonymous closures. Core builds the map of which roles may call which route from the callback's function name, so on PHP 8.2 and 8.3 both routes get the same key, {closure}, and the status route ends up with the resend route's roles. Authors and assistants then get a 401. Using named callables (e.g. $this->getStatus(...)) fixes this.
• "Try again" sets the status to pending in SQL, but edit() then writes "failed" back, because the submission object in memory still says "failed".
• A connection failure (DNS, refused connection, timeout) is reported as "internal server error". Could you report Guzzle's ConnectException (and a RequestException without a response) as "could not connect to OA Switchboard", and log the exception's class and message?
• When scheduling the send fails, Message::sendToOASwitchboard() only logs "OASwitchboard message scheduling failed", without the reason. This includes the case where the plugin isn't configured. Could you include the exception message?
• The acceptance date is only taken from Decision::ACCEPT in external review, so articles accepted with "Accept and Skip Review" (SKIP_EXTERNAL_REVIEW) are sent with an empty acceptance date.
• After a failed Save, the username field shows the stored value (empty on first setup) instead of what was typed, because initData() uses username while readInputData() uses OASUsername.
• Saving the settings without api_key_secret: the form is hidden, but a POST with save=1 still validates the credentials against the API and then fails with a 500. Could you refuse the save before any outside call?
• In 1.1.4.0, use Exception; in a file without a namespace has no effect, and PHP logs a warning about it. This can be removed.
• 1.x/2.x lack the postRequirementsError locale key.

The publication date and the queue-worker issue seem the most important. Would you like to address them before we merge, or should we merge this now and you include them in a next release?

Thanks!

[iudizm]

Hi @bozana, thank you very much for this detailed review.

Yes, I'm addressing these issues and I can update this PR with new releases before merge.

[iudizm]

Hi @bozana,

Thanks again for the review!
I've updated this PR with new releases, which address all your review points above.

A few notes where we went a different way:

• DOIs created at publication: instead of only documenting it, the plugin now listens to the PublicationPublished event, which core dispatches after assigning the DOI, so this case works (3.4 and 3.5). The README also mentions it.
• Queue workers: the send trigger follows your suggestion. For the send status, the plugin now reads and writes its own rows in submission_settings instead of extending the submission schema, so it no longer depends on where the job runs, and core edit() calls can't overwrite it. This also fixes the "Try again" point.
• Repeated sends: it's intended, as OA Switchboard accepts repeated messages for a same DOI. This is now mentioned in the README.

Also fixed a few related issues along the way.

Thanks a lot!

[bozana]

Hi @iudizm,

Thanks a lot for the thorough update! Every point from the first review is fixed or documented in the README. The PublicationPublished listener and storing the status directly work well: DOIs created at publication are now sent, and a command-line queue worker records the status too. We'll merge now.

A few small things for a next release:

1. Password in stack traces: when a send fails, PHP's stack trace shows the arguments, e.g. getAuthorization('…', '<first 15 characters of the password>...'), and core writes it to the error log and to failed_jobs. Could you mark the password and token parameters with #[\SensitiveParameter] (getAuthorization(), sendMessage(), makeRequest())? On PHP 7 the attribute is read as a comment, so it's safe for the 1.x line too.
2. Settings cache after the upgrade migrations: EncryptApiCredentialsMigration and RemoveSandboxApiSettingMigration change plugin_settings with direct SQL — which is correct — but the cached settings then keep the old values (on 3.5 for up to 24 hours). E.g. a journal whose sandbox credentials were removed still counts as configured until then. Adding <code function="clearDataCache" /> after the two migrations in upgrade.xml would let core clear its caches, without any extra code in the migrations.
3. "Not configured" as a normal state: validatePluginIsConfigured() signals a missing configuration by throwing, so the PublicationPublished listener logs "scheduling failed … The OA Switchboard plugin is not configured." on every publish in a journal without credentials, and a resend request answers "internal server error" instead of "not configured". A boolean check up front (e.g. isConfigured(), as the status route effectively does already) would let the listener simply return and the resend route answer with the "not configured" message, leaving the error log and the generic error for real failures.
4. A question: when a new version is published, the message carries that version's publication date. Is that what OA Switchboard expects, or should it be the date of the first publication?

Thanks again