-
Notifications
You must be signed in to change notification settings - Fork 9
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: 15.2.2 release #367
feat: 15.2.2 release #367
Conversation
…es secondary error
… matter default_profile value in global
fix: SODA-Pennsieve key in config.ini becomes default if it exists and is broken
…vanced-features fix: pennsieve agent check in advanced features cannot differentiate uninstalled from not running agent states
fix: ps_upload_to_ds errors before retry vary initialization and caus…
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 version 15.2.2, focusing on bug fixes related to the Pennsieve Agent and improvements to the Advanced Features section. The changes primarily address issues with agent detection, installation checks, and connectivity problems for legacy users. Additionally, there are enhancements to error handling, UI improvements, and minor adjustments to various components. Sequence diagram for Pennsieve Agent Check ProcesssequenceDiagram
participant User
participant UI
participant PennsieveAgent
User->>UI: Click 'Confirm Dataset Manifest'
UI->>PennsieveAgent: Check if installed
alt Agent not installed
UI->>User: Display download link
else Agent installed
UI->>PennsieveAgent: Start check
PennsieveAgent-->>UI: Check successful
UI->>User: Display success message
end
User journey diagram for Advanced Features Updatejourney
title User Journey for Advanced Features Update
section Start Over
User: 5: Click 'Start Over'
System: 5: Hide sections and reset placeholders
section Confirm Dataset Manifest
User: 4: Click 'Confirm Dataset Manifest'
System: 4: Check Pennsieve Agent installation
System: 3: Display appropriate message
section Pull Manifest Information
User: 3: Click 'Pull Manifest Information'
System: 3: Generate manifest folder locally
section Banner Image Upload
User: 2: Navigate to Banner Image Upload
System: 2: Check Pennsieve Agent and transition mode
Class diagram for Pennsieve Agent Error HandlingclassDiagram
class PennsieveAgentErrorMessageDisplay {
+PennsieveAgentErrorMessageDisplay(errorMessage)
+deletePennsieveAgentDBFilesAndRestart()
}
class PennsieveAgentCheckDisplay {
+pennsieveAgentCheckInProgress: bool
+pennsieveAgentCheckError: Error
+pennsieveAgentInstalled: bool
+pennsieveAgentOutputErrorMessage: string
+pennsieveAgentUpToDate: bool
}
PennsieveAgentErrorMessageDisplay --> PennsieveAgentCheckDisplay : 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! |
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 @aaronm-2112 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟡 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.
## Bug Fixes: | ||
|
||
- Fixed an issue with the Pennsieve agent version check not being able to detect old versions of the Pennsieve agent (The agent found at https://github.com/Pennsieve/agent). | ||
- Fixed an issue in how SODA reads the config.ini file that made it so that if the account name 'SODA-Pennsieve' exists it is always used for authentication with Pennsieve even when a defualt_profile key exists in the config. |
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 (documentation): Typo in 'default_profile'
The word 'defualt_profile' is misspelled. It should be 'default_profile'.
@@ -316,3 +328,79 @@ $("#button-import-banner-image").click(async () => { | |||
$("#edit_banner_image_modal").modal("show"); | |||
$("#edit_banner_image_modal").addClass("show"); | |||
}); | |||
|
|||
// Pennsieve Agent check display | |||
document.querySelector("#btn-confirm-dataset-manifest-page").addEventListener("click", async () => { |
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 (complexity): Consider refactoring the Pennsieve Agent check and observer logic into separate, reusable functions.
The recent changes introduce valuable functionality but also increase code complexity. Here are some suggestions to maintain the new features while improving code structure and readability:
- Extract the Pennsieve Agent check into a separate async function:
async function checkPennsieveAgentAndUpdateUI(checkDivId) {
const checkDiv = document.getElementById(checkDivId);
checkDiv.classList.remove("hidden");
try {
return await window.checkPennsieveAgent(checkDivId);
} catch (e) {
console.error("Error with agent:", e);
return false;
}
}
- Create a generic observer function to reduce duplication:
function createSuccessObserver(targetDiv, successText, callback) {
return new MutationObserver((mutations) => {
if (targetDiv.textContent.includes(successText)) {
callback();
observer.disconnect();
}
});
}
- Simplify the event listener for '#btn-confirm-dataset-manifest-page':
document.querySelector("#btn-confirm-dataset-manifest-page").addEventListener("click", async () => {
this.classList.add("hidden");
const checkDivId = "advanced-features-manifest-generation-pennsieve-agent-check";
if (await checkPennsieveAgentAndUpdateUI(checkDivId)) {
document.querySelector("#div-btn-pull-ds-manifest").classList.remove("hidden");
document.querySelector("#div-btn-pull-ds-manifest").scrollIntoView({ behavior: "smooth" });
}
});
- Use the generic observer for both cases:
const agentCheckSuccessText = "The Pennsieve Agent is running and ready to upload!";
createSuccessObserver(
document.querySelector("#advanced-features-manifest-generation-pennsieve-agent-check"),
agentCheckSuccessText,
() => document.querySelector("#div-btn-pull-ds-manifest").classList.remove("hidden")
).observe(targetDiv, { childList: true, subtree: true, characterData: true });
createSuccessObserver(
document.querySelector("#advanced-features-banner-image-pennsieve-agent-check"),
agentCheckSuccessText,
async () => {
await window.transitionFreeFormMode(
document.querySelector("#div_add_edit_banner_image_agent_check"),
"div_add_edit_banner_image_agent_check",
"delete",
"freeform"
);
await window.wait(1000);
document.querySelector("#edit_banner_image_button").scrollIntoView({ behavior: "smooth" });
}
).observe(targetDiv, { childList: true, subtree: true, characterData: true });
These changes will significantly reduce code duplication, improve readability, and make the code more maintainable while preserving all new functionality.
Quality Gate passedIssues Measures |
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
Release version 15.2.2 with multiple bug fixes related to the Pennsieve Agent, including detection of legacy versions, installation checks, and connectivity issues. Enhance error handling and user interface messages for the Pennsieve Agent. Update CI workflows to remove outdated branch references and document changes in the changelog.
New Features:
Bug Fixes:
Enhancements:
CI:
Documentation: