-
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.1.0 #97
Conversation
* 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>
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 pull request implements a new workflow for GitHub release integration with Zenodo archiving, adds support for custom license titles, and includes various improvements to error handling, logging, and UI elements. The changes focus on enhancing the user experience while ensuring proper metadata handling and archival compliance. Sequence diagram for GitHub release integration with Zenodo archivingsequenceDiagram
actor User
participant GitHub
participant Zenodo
participant Database
participant Bot
User->>GitHub: Create a release
GitHub->>Bot: Trigger release event
Bot->>Database: Fetch Zenodo token
Database-->>Bot: Return Zenodo token
Bot->>Zenodo: Create or fetch deposition
Zenodo-->>Bot: Return deposition info
Bot->>GitHub: Update release metadata
Bot->>Zenodo: Upload assets
Bot->>Zenodo: Publish deposition
Zenodo-->>Bot: Confirm publication
Bot->>GitHub: Update release status
Bot->>User: Notify release completion
Updated class diagram for archival and license handlingclassDiagram
class Archival {
+updateGitHubRelease(context, repositoryName, owner, releaseId)
+publishZenodoDeposition(zenodoToken, depositionId)
+getZenodoToken(user)
+parseZenodoInfo(issueBody)
}
class License {
+validateLicense(licenseRequest, existingLicense)
+applyLicenseTemplate(context, owner, repository, subjects)
}
class Metadata {
+getCodemetaContent(context, owner, repository)
+getCitationContent(context, owner, repository)
+validateMetadata(content, fileType)
+updateMetadataIdentifier(context, owner, repository, identifier, version)
}
Archival --> Zenodo
Archival --> GitHub
License --> Database
Metadata --> Database
Metadata --> GitHub
Metadata --> Zenodo
Zenodo : API
GitHub : API
Database : Prisma
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! |
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 and they look great!
Here's what I looked at during the review
- π‘ General issues: 3 issues found
- π‘ Security: 1 issue found
- π’ Testing: all looks good
- π’ Complexity: all looks good
- π‘ Documentation: 1 issue found
Help me be more useful! Please click π or π on each comment and I'll use the feedback to improve your reviews.
} | ||
}); | ||
|
||
if (!deposition || !deposition.token) { |
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 (bug_risk): Error message references undefined variable 'tagVersion'
The error message references 'tagVersion' which is not defined in this scope. Consider updating to reference the 'user' parameter instead.
}); | ||
|
||
if (!deposition || !deposition.token) { | ||
throw new Error(`Deposition with tag ${tagVersion} not found in db.`, { cause: error }); |
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 (bug_risk): Undefined error variable used in error cause
The error cause references an undefined 'error' variable. This could cause unexpected behavior.
if (!match) { | ||
throw new Error("Zenodo publish information not found in issue body."); | ||
} | ||
const [depositionId, releaseId, tagVersion, userWhoSubmitted] = match[1].trim().split(/\s+/); |
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: Add validation for parsed Zenodo info values
Consider adding validation to ensure all required values are present and valid before returning them.
const [depositionId, releaseId, tagVersion, userWhoSubmitted] = match[1].trim().split(/\s+/); | |
const [depositionId, releaseId, tagVersion, userWhoSubmitted] = match[1].trim().split(/\s+/); | |
if (!depositionId || !releaseId || !tagVersion || !userWhoSubmitted) { | |
throw new Error("Missing required Zenodo publish information values"); | |
} |
@@ -118,7 +118,7 @@ export async function applyCWLTemplate( | |||
const privateRepo = await isRepoPrivate(context, owner, repository.name); | |||
const identifier = createId(); | |||
const overallSection = `\n\n## Language Specific Standards\n\nTo make your software FAIR is it important to follow language specific standards and best practices. Codefair will check below that your code complies with applicable standards,`; | |||
let url = `${CODEFAIR_DOMAIN}/view/cwl-validation/${identifier}`; | |||
let url = `${CODEFAIR_DOMAIN}/dashboard/${owner}/${repository.name}/view/cwl-validation`; |
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 (security): Use URL constructor for safer URL building
String concatenation for URLs can be unsafe if variables contain special characters. Consider using the URL constructor or proper URL encoding methods to safely construct URLs.
const url = new URL(`dashboard/${owner}/${repository.name}/view/cwl-validation`, CODEFAIR_DOMAIN).toString();
@@ -5,6 +5,23 @@ All notable changes the Codefair App will be documented in this file. | |||
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) | |||
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). | |||
|
|||
|
|||
## v.3.1.0 - 11-05-2024 |
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 (documentation): Version number format inconsistent with previous entries
Consider removing the extra period to match the format used in other version numbers (e.g., 'v3.1.0')
## v.3.1.0 - 11-05-2024 | |
## v3.1.0 - 11-05-2024 |
const newZenodoDeposition = await createNewZenodoDeposition(zenodoToken); | ||
return newZenodoDeposition; |
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): Inline variable that is immediately returned (inline-immediately-returned-variable
)
const newZenodoDeposition = await createNewZenodoDeposition(zenodoToken); | |
return newZenodoDeposition; | |
return await createNewZenodoDeposition(zenodoToken); | |
Explanation
Something that we often see in people's code is assigning to a result variableand then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
}) | ||
]); | ||
|
||
const license = licenseResponse?.license_id ? 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 = licenseResponse?.license_id ? true : false; | |
const license = !!licenseResponse?.license_id; |
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.
]); | ||
|
||
const license = licenseResponse?.license_id ? true : false; | ||
const citation = metadataResponse?.contains_citation ? 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 citation = metadataResponse?.contains_citation ? true : false; | |
const citation = !!metadataResponse?.contains_citation; |
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.
|
||
const license = licenseResponse?.license_id ? true : false; | ||
const citation = metadataResponse?.contains_citation ? true : false; | ||
const codemeta = metadataResponse?.contains_codemeta ? 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 codemeta = metadataResponse?.contains_codemeta ? true : false; | |
const codemeta = !!metadataResponse?.contains_codemeta; |
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.
const license = licenseResponse?.license_id ? true : false; | ||
const citation = metadataResponse?.contains_citation ? true : false; | ||
const codemeta = metadataResponse?.contains_codemeta ? true : false; | ||
const cwl = cwlResponse?.contains_cwl_files ? 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 cwl = cwlResponse?.contains_cwl_files ? true : false; | |
const cwl = !!cwlResponse?.contains_cwl_files; |
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.
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 a new workflow to streamline GitHub release integration with Zenodo archiving, ensuring metadata is updated before releases. Add support for custom license titles and improve error handling and logging. Fix issues with metadata PR links and codemeta.json date updates. Enhance the UI with additional resources and improved layouts.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Summary by Sourcery
Introduce a new workflow to streamline GitHub release integration with Zenodo archiving, ensuring metadata is updated before releases. Add support for custom license titles and improve error handling and logging. Fix issues with metadata PR links and codemeta.json date updates. Enhance the UI with additional resources and improved layouts.
New Features:
Bug Fixes:
Enhancements:
Documentation: