Add small fix for storeCollectionId #1341
Merged
Conversation
This file contains 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
Reviewer's Guide by SourceryThis pull request introduces a small fix to the Sequence diagram for storeCollectionId function updatesequenceDiagram
actor User
participant WebServer as Web Server
participant Session
User->>WebServer: HTTP Request with collection_id
WebServer->>Session: Set session.collection_id
WebServer->>Session: Set session.collection_type to 'mapped'
WebServer->>User: HTTP Response
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @t-ramz - 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: 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.
nedvedba
approved these changes
Mar 11, 2025
JoshuaSBrown
added a commit
that referenced
this pull request
Mar 11, 2025
* Merge hotfixes into devel (#1326) * [DAPS-DEPS] Update cpp-py-formatter to 0.3.0 * [DAPS-DEPS] Update cpp-py-formatter to 0.3.1 * [DAPS-DEPS] Update ver, add push * updated versions for release * Update project.rst * Update format-check.yml * Pull in version numbers appropriately * Avoid build cache * Aggressive prune * Trigger rebuild * generate_repo_config.sh Accept env variable for datafed server from repo service instead of hard coding * Allow core config thread options to be read in from env * Add changelog comment * Adjust deployment version number * Fix Version month * Update CHANGELOG.md Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> --------- Co-authored-by: Aaron Perez <perezam@ornl.gov> Co-authored-by: Blake Nedved <nedvedba@ornl.gov> Co-authored-by: Joshua S Brown <joshbro42867@yahoo.com> Co-authored-by: JoshuaSBrown <brownjs@ornl.gov> Co-authored-by: Anthony Ramirez <ramirezat@ornl.gov> Co-authored-by: nedvedba <145805866+nedvedba@users.noreply.github.com> Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> * Feature DAPS 1215 foxx UserGetAccessTokenRequest mapped collection support (#1284) * Begin prototyping required code for mapped collection token retrieval. * SDMS_Auth.proto Add optional fields to UserGetAccessTokenRequest and UserAccessTokenReply to support finding correct tokens * user_router.js Add baseline query params and checks. DatabaseAPI.cpp & DatabaseAPI.hpp Use updated names and types. ClientWorker.cpp Update comments. * user_router.js Add logic for finding token based on collection; add token match and missing match cases * Return necessary data from Database to refresh tokens. Mock data on dir/list to test out changes. * user_router.js Add check for existence of Globus collection * user_router.js Remove existence check, move to filter method; Add notes on covered cases; add notes on code * datafed-ws.js Temporary handling of need consent response. * user_router.js Add queryParam validation for new params via joilib. ClientWorker.cpp Add refresh token try/catch to send through flow if refresh fails * datafed-ws.js Use correct converted casing for protobuf messages; redirect to error when needs_consent * datafed-ws.js No redirect, show error directly in list. * datafed-ws.js Early return to prevent further requests * support.js Update AccessTokenType enum. user_token.js New class introduced for handling logic for validating params when getting tokens and building GetAccessToken responses. user_router.js Replace logic with calls to new lib * user_token.js Add jsdoc definition for return type and format function. user_router.js Remove unnecessary comment. * user_token.js More jsdoc definition; Add validation for collection token document required fields. user_token.test.js Add preliminary unit tests for static methods. * CMakeLists.txt Add new unit tests to build. user_token.test.js Modify naming to match convention. * CMakeLists.txt Fix variable get. user_token.js Fix export. * user_fixture.js Add new user for get/token tests. user_router.test.js Add simple tests for get/token endpoint. user_token.js Fix naming for token document scopes field. user_token.test.js Changes for scope field bugfix. * user_router.js Formatting; Throw error if more than one token matches, add note about token type. * ClientWorker.cpp Address some TODOs and extraneous comments. DatabaseAPI.cpp Remove TODOs. SDMS_Auth.proto Remove unused field. * user_token.js Update formatUserToken to always return object values. DatabaseAPI.cpp Add note on possibly missing field. user_token.test.js Update tests to check for new expected values. * user_router.js Clean up some comments for clarity. datafed-ws.js Formatting. * ClientWorker.cpp Rewrite comment regarding mapped token refresh. * user_token.js Formatting. * CHANGELOG.md update for feature * user_router.js Re-introduce accidentally removed `globus_collection` object. * CMakeLists.txt Remove comment about necessity * Update token lookup logic to use `byExample`, add test case for missing Globus collection. Add globus_coll fixture. * SDMS_Auth.proto Change UserAccessTokenReply needs_consent to optional field. * user_router.js Update token/get to default to start consent flow when collection tokens are present. user_router.test.js Update test from error to needs_consent. * Formatting changes. --------- Co-authored-by: Anthony Ramirez <ramirezat@ornl.gov> * [DLT-1110] Implement Consent Required Action (3/4 & 4/4) (#1242) * [DLT-1110] Refactor browse into component and use CSS * [DAPS-1110] Update * [DAPS-1110] Update web server * [DAPS-1110] add refactors * [DAPS-1110] Prettier * [DAPS-1110] Refactor tests * [DAPS-1110] Remove .. in ep ls, remove logs * [DAPS-1110] Add sinon, remove dead code * [DAPS-1110] Address transfer start button bug * [DAPS-1110] eslint, update api.test.js to reflect usage * [DAPS-1110] format * [DAPS-1110] Correct bug * JSON parse error fix (#1328) * DatabaseAPI.cpp Pull in json dependency, use json object to serialize request payload * Replace json serialization in more locations where parseSearchRequest is being called. * Add comments around locations that will need json serialization * Convert missed body append to serialization format * typo * Refactoring DatabaseAPI.cpp to incorporate json serialization * More serialization work * Pull in changes from 1214 for dataPut * Bring in changes from 1214 for dataGet * DatabaseAPI.cpp Fix error with missing curly brace. Some formatting that is likely wrong. Serialize up to original point. Need to validate. * Add missing scope resolution * More typos * More typos. Scoping * Remove non-existant vars * declare body * Formatting * Formatting and verification of updates * Replace most places * Formatting * Remove redeclaration * DatabaseAPI.cpp Remove some comments. * DatabaseAPI.cpp Add missing push_back * Adding braces * Prevent Double Escaping string, by using nlohmann json parse to read in value. * remove escapeJSON calls, redundant with nlohmann json dump * Allow parsing for metric data to be run in parallel for now, output messages when old serialization is used. * Ensure ASCII setting. * Upgrade debug to warning. --------- Co-authored-by: Anthony Ramirez <ramirezat@ornl.gov> Co-authored-by: Austin Hampton <amh107@latech.edu> Co-authored-by: Joshua S Brown <joshbro42867@yahoo.com> * Feature - Mapped Collection Token Support (#1334) * Begin prototyping required code for mapped collection token retrieval. * SDMS_Auth.proto Add optional fields to UserGetAccessTokenRequest and UserAccessTokenReply to support finding correct tokens * user_router.js Add baseline query params and checks. DatabaseAPI.cpp & DatabaseAPI.hpp Use updated names and types. ClientWorker.cpp Update comments. * user_router.js Add logic for finding token based on collection; add token match and missing match cases * Return necessary data from Database to refresh tokens. Mock data on dir/list to test out changes. * user_router.js Add check for existence of Globus collection * user_router.js Remove existence check, move to filter method; Add notes on covered cases; add notes on code * datafed-ws.js Temporary handling of need consent response. * user_router.js Add queryParam validation for new params via joilib. ClientWorker.cpp Add refresh token try/catch to send through flow if refresh fails * datafed-ws.js Use correct converted casing for protobuf messages; redirect to error when needs_consent * datafed-ws.js No redirect, show error directly in list. * datafed-ws.js Early return to prevent further requests * support.js Update AccessTokenType enum. user_token.js New class introduced for handling logic for validating params when getting tokens and building GetAccessToken responses. user_router.js Replace logic with calls to new lib * user_token.js Add jsdoc definition for return type and format function. user_router.js Remove unnecessary comment. * user_token.js More jsdoc definition; Add validation for collection token document required fields. user_token.test.js Add preliminary unit tests for static methods. * CMakeLists.txt Add new unit tests to build. user_token.test.js Modify naming to match convention. * CMakeLists.txt Fix variable get. user_token.js Fix export. * user_fixture.js Add new user for get/token tests. user_router.test.js Add simple tests for get/token endpoint. user_token.js Fix naming for token document scopes field. user_token.test.js Changes for scope field bugfix. * user_router.js Formatting; Throw error if more than one token matches, add note about token type. * ClientWorker.cpp Address some TODOs and extraneous comments. DatabaseAPI.cpp Remove TODOs. SDMS_Auth.proto Remove unused field. * user_token.js Update formatUserToken to always return object values. DatabaseAPI.cpp Add note on possibly missing field. user_token.test.js Update tests to check for new expected values. * user_router.js Clean up some comments for clarity. datafed-ws.js Formatting. * ClientWorker.cpp Rewrite comment regarding mapped token refresh. * user_token.js Formatting. * CHANGELOG.md update for feature * user_router.js Re-introduce accidentally removed `globus_collection` object. * CMakeLists.txt Remove comment about necessity * Add collection_type and collection_id to dat/put task state * support.js Update getAccessToken method to work based on collection information, JSDoc. tasks.js Include context required in call to getAccessToken, add comment on refresh. TaskWorker.cpp Identify where changes will be reflected. * Update token lookup logic to use `byExample`, add test case for missing Globus collection. Add globus_coll fixture. * SDMS_Auth.proto Change UserAccessTokenReply needs_consent to optional field. * user_router.js Update token/get to default to start consent flow when collection tokens are present. user_router.test.js Update test from error to needs_consent. * Formatting changes. * SDMS_Auth.proto Add new fields to DataGetRequest * Begin refactor of token decision logic. * Address bugs in fetching and mapping data, make unit tests pass. * globus_token.js Match other models for get * tasks.js Pull in new logic for determining tokens, replace tokens; Needs to be verified. user_token.js Add new format function for compatibility with transfer doc replacement. * DatabaseAPI.cpp Sane serialization of json body for taskInitDataPut. TaskWorker.cpp Comments around work items. * DatabaseAPI.cpp Sane serialization of json body for taskInitDataGet. * ClientWorker.cpp Stub out required changes for consent flow when creating tasks * ClientWorker.cpp More thorough consent flow for procDataPutRequest, stub on needs_consent * tasks.js Throw error when initializing data put * tasks.js Format error throwing. ClientWorker.cpp comment out potentially unnecessary code. * user_token.js Add exists method to abstract check. tasks.js User exists method when checking token. * user_token.js JSDoc typing * tasks.js Add more required fields to taskRunDataPut for refresh in TaskWorker. * tasks.js Add collection_id to params in taskRunDataPut for refresh in TaskWorker. TaskWorker.cpp Refresh conditionals * DatabaseAPI.cpp Update merged devel changes to address some leftover code. * DatabaseAPI.cpp Add collection specification to get request. * DatabaseAPI.cpp whitespace. ClientWorker.cpp Clean up unused code. user_router.js Switch to checking token existence through standard API. * user_token.js Add comment about type return. data_router.js Do validations at router level for user collection token relationship. tasks.js Remove validation from logic level; make dataGet mirror dataPut; Updates to naming and params structure. * ClientWorker.cpp Revert to minimize diff. TaskWorker.cpp Remove unnecessary TODO; move set client for refresh in transfer tasks. * models/ JSDocs, formatting, make token fetches follow defined model. user_token.js JSDocs and typing. * user_token.js Cover case where collection does not exist. * Small changes for UI detection of collection type * fix epview call for UI changes, add list view change to react appropriately to needing consent according to current code * extract ep view logic to function for either path to utilize * small changes and notes for UI updates * UI search based on path, not ID. camelCasing * TaskWorker.cpp small fix to only enforce requirement of additional refresh fields when token is transfer token. * add params for dat/get on UI * task.js Fix bug where client is incorrectly referenced. * Add user model tests. Extract DataFedOAuthToken.js from user_token.js for reusability. * Add globus collection model tests. * Add globus token tests. Add fixtures to support globus token testing. Make all model properties static for iteration purposes. Utilize constructors for typing. * Add user token tests. Make non-existent user throw an error when building user tokens. * Change error to be not found * Formatting changes * Remove unused changes in support.js getAccessToken * Changelog update, update comments, remove log. --------- Co-authored-by: Anthony Ramirez <ramirezat@ornl.gov> * Add small fix for storeCollectionId (#1341) * Add small fix for storeCollectionId * move todo comment --------- Co-authored-by: Anthony Ramirez <ramirezat@ornl.gov> --------- Co-authored-by: Anthony <64566330+t-ramz@users.noreply.github.com> Co-authored-by: Aaron Perez <perezam@ornl.gov> Co-authored-by: Joshua S Brown <joshbro42867@yahoo.com> Co-authored-by: JoshuaSBrown <brownjs@ornl.gov> Co-authored-by: Anthony Ramirez <ramirezat@ornl.gov> Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> Co-authored-by: Aaron Perez <aperez0295@gmail.com> Co-authored-by: Austin Hampton <amh107@latech.edu>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
PR Description
Tasks
Summary by Sourcery
Bug Fixes: