-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: ✨ Codefair v3.2.0 #115
Conversation
* merge: 🔀 merge main to staging * refactor: ♻️ 🐛 Codefair 3.1.0 (#89) * fix: 🐛 patch removing pull_request.closed bug on metadata pr merge * fix: 🐛 don't update firstPublished in the codemeta file + better logs * fix: 🐛 patch error on reading db entry when it doesn't exist * chore: 🔊 better log on successful Zenodo publish * style: 🎨 margins between text and icon * feat: ✨ links to the list of tools used for codefair in /codefair * refactor: ✨ ♻️ abstract the license validation fn * doc: 🔖 update changelog * feat: ✨ ♻️ abstraction to zenodo workflow * wip: 🚧 abstracting zenodo workflow * feat: ✨ add help link in the Zenodo release page * refactor: ♻️ ✨ abstract out github release functions * refactor: 🔊 ♻️ improve error tracing in metadata file * chore: 🔨 remove todo * refactor: ✨ ♻️ abstracted Zenodo workflow (bot) * wip: 🚧 save custom license content when no assertion is provided * ✨ feat: add support for custom licenses (#90) * ✨ feat: add support for custom licenses * 🐛 fix: reset citation license field * 🐛 fix: update zenodo workflow for custom licenses * fix: 🐛 license is valid with custom license * feat: ✨ custom license template * ✨ feat: update codemeta, license and cwl ui paths (#91) * ✨ feat: update meta paths for consistency * 🚚 fix: updatel icense request path * ✨ feat: add support for custom license title (#92) * refactor: ♻️ ✨ new patches for license, cwl, metadata UI's * refactor: ✨ ♻️ apply migrations from UI to bot * feat: ✨ request custom license title from user * refactor: ♻️ hide message box when custom license title is blank * refactor: ♻️ issue dashboard requests custom title * feat: ✨ Saving custom title (#93) * feat: ✨ save custom title option * refactor: :recyle: reuse saveLicenseDraft for storing custom license title * refactor: ♻️ join draft and save title buttons together * refactor: ♻️ use naive-ui's properties for styling * chore: 🔨 remove typo * chore: 🔨 remove unused variables * feat: ✨ toasts for custom title saved button * refactor: ♻️ function rename * feat: ✨ create Zenodo metadata fn handles custom licenses * feat: ✨ custom license reference for archival workflow * refactor: ♻️ stop creating identifiers for new db entries * refactor: ♻️ 🐛 revert allowing custom licenses for zenodo * refactor: ♻️ Custom License Support (#94) * refactor: ♻️ ✨ custom title saved updates dashboard, disable create release btn when custom license * refactor: ♻️ block progress of fair release if license is custom * feat: ✨ listener for when custom license title is saved through UI * feat: ✨ prevent fair release if not fair compliant * refactor: ♻️ improve license validation and update template text for custom licenses * refactor: ♻️ re-render issue from ui side * refactor: ♻️ update to message explaining custom licenses * db calls running in parallel * fix: 🐛 prevent Zendo section from being seen when the license is custom * fix: 🐛 trim license content before comparison --------- Co-authored-by: Sanjay Soundarajan <sanjay.16@live.com> * refactor: ♻️ safety checks for protected middleware * refacotr: ♻️ update protectRoute to redirect to login when no user or session * feat: :fix: redirect to login if not signed in * refactor: ♻️ remove import * chore: 🔊 🔨 remove logs * docs: 📝 update changelog * fix: 🐛 check license content all times * fix: 🐛 update on zenodo ui page * feat: ✨ delete branch after pr has been closed/merged * chore: 🔨 spacing in changelog * wip: 🚧 individual validation requests for license and metadata * feat: ✨ license validation endpoint * wip: 🚧 creating updated metadata validation * wip: 🚧 patches for metadata revalidation workflow * wip: 🚧 validation for codemeta * doc: 🔖 jsdoc comments on fns * fix: 🐛 template renderer will stop using old links (#99) * fix: 🐛 rerender template will stop using old links * doc: 📝 update CHANGELOG * doc: 📝 update metadata * fix: 🐛 preserve authors and contributors from codemeta * feat: ✨ re-validated codemeta from repo dashboard * fix: 🐛 await metadata validation * feat: ✨ codemeta validation extended with microservice validator * refactor: ♻️ update timestamps to be unix format * feat: ✨ clean privatekey env before use * refactor: ♻️ update cwl validation endpoint * wip: 🚧 send validation message to db * feat: ✨ create migrations for metadata validation messages * wip: 🚧 creating json schema for codemeta.json validations * 👷 ci: setup deployment environments (#101) * 👷 ci: setup for staging env (#103) Co-authored-by: Sanjay Soundarajan <sanjay.16@live.com> Co-authored-by: slugb0t <wheresdorian@gmail.com> fix: 🐛 template renderer will stop using old links (#99) * fix: 🐛 adjust env variables used after env changes * 👷 ci: cleanup dockerfile * feat: ✨ codemeta schema first draft * 👷 ci: cleanup dockerfile * 👷 ci: cleanup dockerfile * 👷 ci: cleanup dockerfile * 👷 ci: setup for staging env * 👷 ci: setup for staging env * 👷 ci: setup for staging env * 👷 ci: setup for staging env * 👷 ci: cleanup dockerfile * 👷 ci: fix staging deploys (#104) * wip: 🚧 abstracting cwl workflow * refactor: ♻️ update codemeta schema to validate against the raw codemeta.json files * refactor: ♻️ update codemeta schema for additional fields * feat: ✨ validating raw content of codemeta.json file for 3.0 * refactor: ♻️ create required fields for codemeta schema * wip: 🚧 testing validation responses for db * handle codemeta.json 3.0 and 2.0 versioning * fix: 🐛 correct branch watch for staging deployment action * doc: 🔖 update changelog for 3.2.0 release * feat: ✨ align bot prisma with ui * fix: 🐛 bug patch for cwl workflow * feat: ✨ metadata workflow update * refactor: ♻️ preserving authors and contributors * wip: 🚧 update the metadata workflow after discussions * fix: 🐛 don't create cwlobject twice * refactor: ♻️ send 200 status when invalid codemeta * refactor: ♻️ update codemeta validation schema * refactor: ♻️ do not revalidate metadata files for push events unless metadata files are updated themselves * wip: 🚧 seperate rerun validation with regather information * wip: 🚧 remove command from issue dashboard if error occurs (allows retry) * refactor: ♻️ update the issue body after successful validation (remove the command from the issue body) * refactor: ♻️ add additional key to codemeta schema * refactor: ♻️ remove the regather options from the UI as validation is still needed when regathering * fix: 🐛 pr button updated with new links * fix: 🐛 patch variable declaration not in scope * refactor: ♻️ update on the dropdown icons * refactor: ♻️ update the keys of the cwl object * refactor: ♻️ update the getcwlfiles function --------- Co-authored-by: Sanjay Soundarajan <sanjay.16@live.com>
* fix: 🐛 correct cwlObject variables in push event * fix: 🐛 better error message in metadata workflow + collect missing fields from codemeta.json * fix: 🐛 apply await to async function call * refactor: ♻️ removed unused imports * refactor: ♻️ add try catch in then statement of promise * fix: 🐛 missing fields in codemeta gathering + add error handling for convertCitationForDB fn * refactor: ♻️ add try catch to renderer to preserve errors * fix: 🐛 preserve metadata validation results in db * wip: 🚧 validation results being displayed through the repo dashboard * wip: 🚧 displaying metadata validation results in seperate pages * feat: ✨ validation page for codemeta * fix: 🐛 patch fundingCode not being preserved from codemeta * wip: 🚧 final design for validation results * feat: ✨ add view validation results conditionally * test: ⚗️ ensuring validation progresses accordingly * feat: ✨ popover messages for metadata badges * feat: ✨ if metadata file fails to parse then update validation message * fix: 🐛 update schema to handle 2.0 and 3.0 authors * style: 🎨 formatting * fix: 🐛 add releaseNotes key to schema for 3.0 versions * refactor: ✏️ change wording * fix: 🐛 validate metadata file that was updated only on push events * chore: 📝 remove test logs
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 Guide by SourceryThis PR implements comprehensive metadata validation capabilities, including new validation endpoints for codemeta.json and CITATION.cff files, enhanced UI features for displaying validation results, and improved error handling and logging functionality. Sequence diagram for metadata validation processsequenceDiagram
participant User
participant UI
participant Server
participant MetadataValidator
participant Database
User->>UI: Request metadata validation
UI->>Server: Send validation request
Server->>MetadataValidator: Validate metadata
MetadataValidator->>Database: Fetch existing metadata
Database-->>MetadataValidator: Return metadata
MetadataValidator->>Database: Update validation status
Database-->>MetadataValidator: Confirmation
MetadataValidator-->>Server: Validation result
Server-->>UI: Display validation result
UI-->>User: Show validation status
Class diagram for metadata validation functionsclassDiagram
class MetadataValidator {
+validateMetadata(metadataInfo, fileType, repository)
+applyDbMetadata(existingMetadataEntry, metadata)
+applyCodemetaMetadata(codemeta, metadata, repository)
+applyCitationMetadata(citation, metadata, repository)
}
class CodemetaConverter {
+convertCodemetaForDB(codemetaContent, repository)
}
class CitationConverter {
+convertCitationForDB(citationContent, repository)
}
class Database {
+update(metadata)
+findUnique(query)
}
MetadataValidator --> CodemetaConverter
MetadataValidator --> CitationConverter
MetadataValidator --> Database
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! |
PR Summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @slugb0t - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider standardizing the error handling pattern across the codebase - in some places errors are logged and rethrown while in others they are just logged. A consistent approach would make the code more maintainable.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟡 Security: 2 issues found
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
"error": "Unsupported codemeta version", | ||
}, 400 | ||
try: | ||
with open("./codemeta-schema.json", "r", encoding="utf-8") as f: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚨 issue (security): Use absolute path resolution for file access
Using relative paths for file access can be unsafe. Consider using path.join() with __dirname or an absolute path to ensure reliable and secure file resolution.
* @param {string} endpoint - Log endpoint URL | ||
*/ | ||
constructor(endpoint) { | ||
const BOT_ENDPOINT = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚨 issue (security): Avoid hardcoded fallback endpoint URL
Using a hardcoded endpoint URL as fallback could lead to unintended data exposure. Consider requiring an explicit endpoint URL.
} | ||
}); | ||
|
||
const license = licenseRequest.data.license ? true : false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code-quality): Avoid unneeded ternary statements (simplify-ternary
)
const license = licenseRequest.data.license ? true : false; | |
const license = !!licenseRequest.data.license; |
Explanation
It is possible to simplify certain ternary statements into either use of an||
or !
.This makes the code easier to read, since there is no conditional logic.
if (metadata.authors) { | ||
if (convertedCodemeta.authors.length > 0) { | ||
const updatedAuthors = convertedCodemeta.authors.map((author) => { | ||
// Find a matching author in metadata | ||
const foundAuthor = metadata.authors.find( | ||
(existingAuthor) => | ||
existingAuthor?.familyName === author?.familyName && | ||
existingAuthor?.givenName === author?.givenName, | ||
); | ||
|
||
if (foundAuthor) { | ||
// Merge roles, avoiding duplicates based on `role` and `startDate` | ||
if (!foundAuthor?.roles) { | ||
foundAuthor.roles = []; | ||
} | ||
const mergedRoles = [ | ||
...foundAuthor.roles, | ||
...author.roles.filter( | ||
(newRole) => | ||
!foundAuthor.roles.some( | ||
(existingRole) => | ||
existingRole.role === newRole.role && | ||
existingRole.startDate === newRole.startDate, | ||
), | ||
), | ||
]; | ||
|
||
// Merge and prioritize data from `author` | ||
return { | ||
...foundAuthor, | ||
...author, | ||
affiliation: author.affiliation || foundAuthor.affiliation || "", | ||
email: author.email || foundAuthor.email || "", | ||
uri: author.uri || foundAuthor.uri || "", | ||
roles: mergedRoles, | ||
}; | ||
} | ||
|
||
// If no match, return the current author from convertedCodemeta | ||
return author; | ||
}); | ||
|
||
// Merge updated authors with any authors in metadata not present in convertedCodemeta | ||
const nonUpdatedAuthors = metadata.authors.filter( | ||
(existingAuthor) => | ||
!convertedCodemeta.authors.some( | ||
(author) => | ||
author.familyName === existingAuthor.familyName && | ||
author.givenName === existingAuthor.givenName, | ||
), | ||
); | ||
|
||
metadata.authors = [...nonUpdatedAuthors, ...updatedAuthors]; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code-quality): Merge nested if conditions (merge-nested-ifs
)
if (metadata.authors) { | |
if (convertedCodemeta.authors.length > 0) { | |
const updatedAuthors = convertedCodemeta.authors.map((author) => { | |
// Find a matching author in metadata | |
const foundAuthor = metadata.authors.find( | |
(existingAuthor) => | |
existingAuthor?.familyName === author?.familyName && | |
existingAuthor?.givenName === author?.givenName, | |
); | |
if (foundAuthor) { | |
// Merge roles, avoiding duplicates based on `role` and `startDate` | |
if (!foundAuthor?.roles) { | |
foundAuthor.roles = []; | |
} | |
const mergedRoles = [ | |
...foundAuthor.roles, | |
...author.roles.filter( | |
(newRole) => | |
!foundAuthor.roles.some( | |
(existingRole) => | |
existingRole.role === newRole.role && | |
existingRole.startDate === newRole.startDate, | |
), | |
), | |
]; | |
// Merge and prioritize data from `author` | |
return { | |
...foundAuthor, | |
...author, | |
affiliation: author.affiliation || foundAuthor.affiliation || "", | |
email: author.email || foundAuthor.email || "", | |
uri: author.uri || foundAuthor.uri || "", | |
roles: mergedRoles, | |
}; | |
} | |
// If no match, return the current author from convertedCodemeta | |
return author; | |
}); | |
// Merge updated authors with any authors in metadata not present in convertedCodemeta | |
const nonUpdatedAuthors = metadata.authors.filter( | |
(existingAuthor) => | |
!convertedCodemeta.authors.some( | |
(author) => | |
author.familyName === existingAuthor.familyName && | |
author.givenName === existingAuthor.givenName, | |
), | |
); | |
metadata.authors = [...nonUpdatedAuthors, ...updatedAuthors]; | |
} | |
} | |
if (metadata.authors && convertedCodemeta.authors.length > 0) { | |
const updatedAuthors = convertedCodemeta.authors.map((author) => { | |
// Find a matching author in metadata | |
const foundAuthor = metadata.authors.find( | |
(existingAuthor) => | |
existingAuthor?.familyName === author?.familyName && | |
existingAuthor?.givenName === author?.givenName, | |
); | |
if (foundAuthor) { | |
// Merge roles, avoiding duplicates based on `role` and `startDate` | |
if (!foundAuthor?.roles) { | |
foundAuthor.roles = []; | |
} | |
const mergedRoles = [ | |
...foundAuthor.roles, | |
...author.roles.filter( | |
(newRole) => | |
!foundAuthor.roles.some( | |
(existingRole) => | |
existingRole.role === newRole.role && | |
existingRole.startDate === newRole.startDate, | |
), | |
), | |
]; | |
// Merge and prioritize data from `author` | |
return { | |
...foundAuthor, | |
...author, | |
affiliation: author.affiliation || foundAuthor.affiliation || "", | |
email: author.email || foundAuthor.email || "", | |
uri: author.uri || foundAuthor.uri || "", | |
roles: mergedRoles, | |
}; | |
} | |
// If no match, return the current author from convertedCodemeta | |
return author; | |
}); | |
// Merge updated authors with any authors in metadata not present in convertedCodemeta | |
const nonUpdatedAuthors = metadata.authors.filter( | |
(existingAuthor) => | |
!convertedCodemeta.authors.some( | |
(author) => | |
author.familyName === existingAuthor.familyName && | |
author.givenName === existingAuthor.givenName, | |
), | |
); | |
metadata.authors = [...nonUpdatedAuthors, ...updatedAuthors]; | |
} | |
Explanation
Reading deeply nested conditional code is confusing, since you have to keep track of whichconditions relate to which levels. We therefore strive to reduce nesting where
possible, and the situation where two
if
conditions can be combined usingand
is an easy win.
try: | ||
file_content = json.loads(file_content) | ||
except Exception as e: | ||
return { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (code-quality): We've found these issues:
- Use f-string instead of string concatenation [×3] (
use-fstring-for-concatenation
) - Simplify conditional into switch-like form [×3] (
switch
)
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! |
Summary by Sourcery
Introduce metadata and license validation capabilities, enhance the dashboard UI, and integrate the validator service with the Codefair app. Add support for validating codemeta.json and CITATION.cff files using a new schema and validation endpoints. Improve metadata handling and set up deployment configurations for the validator service using Kamal.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Deployment:
Documentation: