Conversation
* fix: 🐛 docker volume for postgres data * feat: ✨ Abstract bot event listeners to files * fix: 🐛 improve SPDX validation and add verification workflow * feat: ✨ detect existing archivals for fair release status * refactor: ♻️ Add line break after pr badge in issue dashboard * fix: 🐛 add await to db call * refactor: ♻️ Sourcery suggestion * fix(sourcery): 🐛 Edge case with doi format and badge generation
|
Thank you for submitting this pull request! We appreciate your contribution to the project. Before we can merge it, we need to review the changes you've made to ensure they align with our code standards and meet the requirements of the project. We'll get back to you as soon as we can with feedback. Thanks again! |
Reviewer's GuideThis PR refactors the Probot bot into modular event handlers, enhances FAIR archival and licensing logic (both backend and dashboard UI) including DOI/Zenodo handling and SPDX-aware license validation, and updates Docker and validator assets to support new behavior and schemas. Sequence diagram for applyArchivalTemplate with metadata DOI detectionsequenceDiagram
participant GH as GitHub
participant Bot as Probot_app
participant Handler as issues_reopened_handler
participant RC as runComplianceChecks
participant Renderer as renderIssues
participant Arch as applyArchivalTemplate
participant Meta as metadata_identifier_helpers
participant DB as Prisma_db
participant Zenodo as Zenodo_API
GH->>Bot: issues.reopened
Bot->>Handler: handleIssuesReopened(context)
Handler->>DB: installation.update(disabled=false)
Handler->>Handler: isRepoEmpty(context, owner, repo)
alt repo_not_empty
Handler->>Handler: gatherCommitDetails(context, owner, repo)
else repo_empty
Handler->>Handler: createEmptyCommitInfo()
end
Handler->>DB: verifyInstallationAnalytics(context, repo, 0, latestCommitInfo)
Handler->>RC: runComplianceChecks(context, owner, repo)
RC-->>Handler: subjects
Handler->>Renderer: renderIssues(context, owner, repo, emptyRepo, subjects)
activate Renderer
Renderer->>Arch: applyArchivalTemplate(context, baseTemplate, repo, owner, subjects)
activate Arch
Arch->>DB: zenodoDeposition.findUnique(repository_id)
alt existing_Codefair_release_with_doi
Arch->>DB: zenodoDeposition.update(existing_zenodo_deposition_id=true)
Arch-->>Renderer: baseTemplate_with_Zenodo_badge
else no_Codefair_release_or_no_doi
Arch->>Meta: fetchAndExtractIdentifiers(context, owner, repo)
activate Meta
Meta->>Meta: getCodemetaContent(context, owner, repo)
Meta->>Meta: extractIdentifiersFromCodemeta(codemetaContent)
Meta->>Meta: getCitationContent(context, owner, repo)
Meta->>Meta: extractIdentifiersFromCitation(citationContent)
Meta->>Meta: prioritizeIdentifiers(identifiers)
Meta-->>Arch: primary_identifier, other_identifiers
alt no_identifiers
Arch-->>Renderer: baseTemplate_with_first_release_cta
else single_identifier
Arch-->>Renderer: baseTemplate_with_single_identifier_section
else multiple_identifiers
Arch-->>Renderer: baseTemplate_with_multiple_identifiers_section
end
end
deactivate Arch
Renderer-->>Handler: final_issue_body
deactivate Renderer
Handler->>GH: createIssue(context, owner, repo, ISSUE_TITLE, issueBody)
GH-->>Zenodo: later_via_publishToZenodo_on_command
Class diagram for archival and license validation helpersclassDiagram
class ArchivalModule {
+IDENTIFIER_TYPE_ZENODO_DOI
+IDENTIFIER_TYPE_OTHER_DOI
+IDENTIFIER_TYPE_NON_DOI
+extractDOIFromString(value)
+classifyIdentifier(identifier)
+extractIdentifiersFromCodemeta(codemetaContent)
+extractIdentifiersFromCitation(citationContent)
+fetchAndExtractIdentifiers(context, owner, repository)
+prioritizeIdentifiers(identifiers)
+createShieldsLabelFromDOI(doi)
+createZenodoDOIBadge(doi, zenodoId)
+createOtherDOIBadge(doi)
+renderSingleIdentifierTemplate(identifier, releaseBadge, firstReleaseBadge)
+renderMultipleIdentifiersTemplate(primary, others, releaseBadge, firstReleaseBadge)
+applyArchivalTemplate(context, baseTemplate, repository, owner, subjects)
}
class LicenseModule {
+isValidSpdxLicense(licenseId)
+normalizeContent(content)
+validateLicense(license, existingLicense)
+updateLicenseDatabase(repository, license)
+applyLicenseTemplate(baseTemplate, repository, subjects, licenseRow)
}
class RendererModule {
+renderIssues(context, owner, repository, emptyRepo, subjects)
+createIssue(context, owner, repository, issueTitle, issueBody)
}
class ActionsModule {
+reRenderDashboard(context, owner, repository, issueBody)
+publishToZenodo(context, owner, repository, issueBody)
}
class HelpersModule {
+ISSUE_TITLE
+PR_TITLES_license
+PR_TITLES_metadataAdd
+PR_TITLES_metadataUpdate
+createEmptyCommitInfo()
}
class InstallationHandlers {
+registerInstallationHandlers(app, db)
}
class PushHandlers {
+registerPushHandler(app, db)
}
class PullRequestHandlers {
+registerPullRequestHandlers(app, db)
}
class IssueHandlers {
+registerIssueHandlers(app, db)
}
class DB_Prismaclient {
+installation
+licenseRequest
+codeMetadata
+zenodoDeposition
}
ArchivalModule --> DB_Prismaclient : uses
ArchivalModule --> HelpersModule : uses_constants
LicenseModule --> DB_Prismaclient : uses
LicenseModule --> ArchivalModule : uses_isValidSpdxLicense
RendererModule --> ArchivalModule : calls_applyArchivalTemplate
RendererModule --> LicenseModule : calls_applyLicenseTemplate
ActionsModule --> RendererModule : uses
InstallationHandlers --> RendererModule : uses
InstallationHandlers --> ArchivalModule : uses_applyArchivalTemplate_indirectly
PushHandlers --> RendererModule : uses
PullRequestHandlers --> ActionsModule : uses_reRenderDashboard
IssueHandlers --> ActionsModule : uses
IssueHandlers --> RendererModule : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Thanks for making updates to your pull request. Our team will take a look and provide feedback as soon as possible. Please wait for any GitHub Actions to complete before editing your pull request. If you have any additional questions or concerns, feel free to let us know. Thank you for your contributions! |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- On
ui/pages/dashboard/[owner]/[repo]/edit/license.vue, then-selectfor license selection was changed fromv-model:value="licenseId"to a one-way binding:value="licenseId"without an@update:valuehandler, which means user selections will no longer updatelicenseIdor triggerupdateLicenseContent; consider restoringv-model:valueor wiring@update:value="updateLicenseContent"(and updatinglicenseIdthere). - The new data-driven command routing in
handlers/issue.jsreturns after the first matched trigger, whereas the previous implementation could execute multiple commands if multiple markers were present in the issue body; if workflows rely on multiple actions being triggered by a single issue edit, you may want to preserve the old behavior by removing the earlyreturnand allowing the loop to continue.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- On `ui/pages/dashboard/[owner]/[repo]/edit/license.vue`, the `n-select` for license selection was changed from `v-model:value="licenseId"` to a one-way binding `:value="licenseId"` without an `@update:value` handler, which means user selections will no longer update `licenseId` or trigger `updateLicenseContent`; consider restoring `v-model:value` or wiring `@update:value="updateLicenseContent"` (and updating `licenseId` there).
- The new data-driven command routing in `handlers/issue.js` returns after the first matched trigger, whereas the previous implementation could execute multiple commands if multiple markers were present in the issue body; if workflows rely on multiple actions being triggered by a single issue edit, you may want to preserve the old behavior by removing the early `return` and allowing the loop to continue.
## Individual Comments
### Comment 1
<location path="bot/index.js" line_range="46-50" />
<code_context>
- });
- }
- });
+ // Register all event handlers
+ registerInstallationHandlers(app, db);
+ registerPushHandler(app, db);
+ registerPullRequestHandlers(app, db);
+ registerIssueHandlers(app, db);
};
</code_context>
<issue_to_address>
**issue (bug_risk):** The `db` variable passed into handler registration is undefined; likely should be `dbInstance`.
`dbInstance` is imported in this module, but the handler registrations call `register*Handlers(app, db)` where `db` is not defined in scope. This will cause a `ReferenceError` on startup. These calls should use `dbInstance` instead of `db`.
</issue_to_address>
### Comment 2
<location path="bot/handlers/issue.js" line_range="123-87" />
<code_context>
+ }
+ }
+
+ // Data-driven command routing with early return
+ for (const { trigger, handler } of COMMANDS) {
+ if (issueBody.includes(trigger)) {
+ await handler(context, owner, repository, issueBody);
+ return;
+ }
+ }
</code_context>
<issue_to_address>
**question (bug_risk):** Early return on the first matching trigger changes behavior from running multiple commands per edit to only one.
The previous handler ran all matching commands because each `if` was independent. With the loop returning after the first match, only one command runs even if the body contains multiple triggers. If batching multiple commands in a single edit is expected, consider dropping the `return` and letting the loop continue, or collecting and running all matching handlers in sequence.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+46
to
+50
| // Register all event handlers | ||
| registerInstallationHandlers(app, db); | ||
| registerPushHandler(app, db); | ||
| registerPullRequestHandlers(app, db); | ||
| registerIssueHandlers(app, db); |
There was a problem hiding this comment.
issue (bug_risk): The db variable passed into handler registration is undefined; likely should be dbInstance.
dbInstance is imported in this module, but the handler registrations call register*Handlers(app, db) where db is not defined in scope. This will cause a ReferenceError on startup. These calls should use dbInstance instead of db.
| logwatch.info( | ||
| "issues.edited: Issue title is not FAIR Compliance Dashboard or the editor is not the bot, ignoring..." | ||
| ); | ||
| return; |
There was a problem hiding this comment.
question (bug_risk): Early return on the first matching trigger changes behavior from running multiple commands per edit to only one.
The previous handler ran all matching commands because each if was independent. With the loop returning after the first match, only one command runs even if the body contains multiple triggers. If batching multiple commands in a single edit is expected, consider dropping the return and letting the loop continue, or collecting and running all matching handlers in sequence.
|
Thanks for closing this pull request! If you have any further questions, please feel free to open a new issue. We are always happy to help! |
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.
Summary by Sourcery
Refactor the bot’s event handling into dedicated handler modules, enhance FAIR archival and license validation logic using metadata-derived identifiers and SPDX-aware checks, and improve the license editing UI/flow along with minor build and Docker configuration updates.
New Features:
Bug Fixes:
Enhancements:
Build: