feat: guest contributor support in SDOA's end to end curation mode #376
Conversation
feat: 15.3.0
…ihub/SODA-for-SPARC into guest-contributor-fixes
…ihub/SODA-for-SPARC into guest-contributor-fixes
…ihub/SODA-for-SPARC into guest-contributor-fixes
|
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 guest contributor support in SDOA's end-to-end curation mode. The changes focus on handling user roles and permissions, particularly for guest editors, and includes modifications to dataset metadata management and UI flow control based on user roles. Sequence diagram for checking user role and skipping pagessequenceDiagram
participant User
participant UI
participant API
participant Client
User->>UI: Clicks on a page
UI->>API: getDatasetRole(dataset-name)
API-->>UI: Returns role
alt User is editor
UI->>UI: Set ineligible to true
UI-->>User: Skip page
else User is not editor
UI->>Client: getUserInformation()
Client-->>UI: Returns user info
UI->>Client: get user organizations
Client-->>UI: Returns organizations
alt User is guest
UI->>UI: Skip permissions tab
else User is not guest
UI->>UI: Unskip permissions tab
end
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
aaronm-2112
requested review from
JacobiClark
and removed request for
bvhpatel
|
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 @aaronm-2112 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider removing debug console.log statements before merging (e.g. 'DOing this resume logic', 'About to save progress', etc)
- There's significant code duplication in the role-checking logic across multiple functions (guidedAddDatasetTags, guidedAddDatasetLicense, etc). Consider extracting this into a reusable helper function
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 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.
| } | ||
| } | ||
|
|
||
| if (ineligible) return { shouldShow: false }; |
There was a problem hiding this comment.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces)
| if (ineligible) return { shouldShow: false }; | |
| if (ineligible) { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
| } | ||
| } | ||
|
|
||
| if (ineligible) return; |
There was a problem hiding this comment.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces)
| if (ineligible) return; | |
| if (ineligible) { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
| } | ||
| } | ||
|
|
||
| if (ineligible) return; |
There was a problem hiding this comment.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces)
| if (ineligible) return; | |
| if (ineligible) { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
| } | ||
| } | ||
|
|
||
| if (ineligible) return; |
There was a problem hiding this comment.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces)
| if (ineligible) return; | |
| if (ineligible) { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
| } | ||
| } | ||
|
|
||
| if (ineligible) return; |
There was a problem hiding this comment.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces)
| if (ineligible) return; | |
| if (ineligible) { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
| } | ||
| } | ||
|
|
||
| if (ineligible) return; |
There was a problem hiding this comment.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces)
| if (ineligible) return; | |
| if (ineligible) { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
…skip permissions page
|
Summary by Sourcery
Add support for guest contributors in SDOA's end-to-end curation mode, allowing them to bypass certain permission steps. Enhance role checking logic to prevent ineligible actions for users with specific roles. Improve logging for better dataset management insights and update CI workflows to accommodate new branch naming.
New Features:
Enhancements:
CI: