feat: Add CODECHECK badge to issue table of contents #83
Conversation
Display badge for articles with completed CODECHECK using Templates::Issue::Issue::Article hook and Smarty template.
nuest
left a comment
nuest
left a comment
There was a problem hiding this comment.
One related question (please turn into issues), one actual question, and one actionable content. Please ping me when updated!
| error_log("CODECHECK: Dropped old table"); | ||
| // Only create table if it doesn't exist | ||
| if (!Schema::hasTable('codecheck_metadata')) { | ||
| error_log("CODECHECK: Creating codecheck_metadata table"); |
There was a problem hiding this comment.
Is there an alternative log function without a notion of "error" ?
|
|
||
| Schema::create('codecheck_metadata', function (Blueprint $table) { | ||
| $table->bigInteger('submission_id')->primary(); | ||
| $table->string('version', 50)->default('latest'); |
There was a problem hiding this comment.
I know this is unrelated to this PR, so independent from the review/fixes, but please clarify: is this the version of the specification that is used? I think the field name should be more informative: spec_version
If this is indeed the specification version, then please open a new issue to capture the task to rename the field.
|
|
||
| error_log("CODECHECK: Table created successfully"); | ||
| } else { | ||
| error_log("CODECHECK: Table already exists, skipping creation"); |
There was a problem hiding this comment.
What about the migrations, for example if a field has a different name or a field was added? Are there mechanisms to handle that, or do we need to do this ourselves here? At the moment we don't need migrations, we can just rename, just wondering for the future if we need to do something more complex in this else clause.
| } | ||
|
|
||
| // If it's a CODECHECK ID, build the URL | ||
| if (preg_match('/^CODECHECK-\d{4}-\d+$/', $certificate)) { |
There was a problem hiding this comment.
When are IDs saved with the preprended string CODECHECK- ? Is this an internal mechanism?
I would prefer to only store the certificate identifier as YYYY-NNN.
| return 'https://doi.org/' . $matches[3]; | ||
| } | ||
|
|
||
| return ''; |
There was a problem hiding this comment.
Can we add a detailed error log for this case? Maybe even alert the user?
We normally want a DOI for all checks, so when this code is reached, something is incomplete in the metadata.
Why not return the content of $report as a fallback? Then the user can see the bad metadata and fix it.
| return 'CODECHECK ' . str_replace('certificate-', '', $lastPart); | ||
| } | ||
|
|
||
| return 'View Certificate'; |
There was a problem hiding this comment.
This string needs to be localised. Maybe you can instead define the link text in the template and the language files?
Display badge for articles with completed CODECHECK using Templates::Issue::Issue::Article hook and Smarty template.
Closes #15