PTV-1913 force cookie resetting, including backup cookie #3657
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3657 +/- ##
===========================================
+ Coverage 25.89% 25.93% +0.04%
===========================================
Files 461 461
Lines 46652 46653 +1
===========================================
+ Hits 12079 12099 +20
+ Misses 34573 34554 -19
... and 5 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
| // const options = { | ||
| // method: callParams.method, | ||
| // headers: { | ||
| // Authorization: token, | ||
| // 'Content-Type': 'application/json', | ||
| // }, | ||
|
|
||
| // } | ||
| // return fetch(callString, options); | ||
|
|
There was a problem hiding this comment.
The only vestige of jquery is that API call wrapped in a Promise, so I started replacing it with a fetch call. Then realized there's some downstream unpacking effects to manage. Then realized that was a silly exercise and wasn't solving anything useful so just put it back and stopped.
…up-cookie' into PTV-1913-add-backup-cookie
|
@ialarmedalien I think this is ready for a real review now. |
There was a problem hiding this comment.
Looks good to me, but I'll leave it up to @ialarmedalien for final review as my narrative-familiarity is low. I am curious about the removal of 'use strict' as it can change error handling and variable declaration handling, but I assume it's safe...
|
@dauglyon the removal of 'use strict' comes from eslint. Compilers add it to builds, so it's not entirely necessary anymore in individual modules. ECMAScript modules are treated as strict by default: from https://eslint.org/docs/latest/rules/strict
The narrative codebase is on the old side, however, so now I'm thinking might not be entirely accurate? If that rule only applies to ECMAScript modules, and we're using AMDs still, is that just the eslint rule being finicky? Now I'm not sure! |
|
It doesn't hurt anything being there, so I think I'll adjust the eslint rule to be safe. I really can't wait to rewrite this stuff to be more modern... |
|
Description of PR purpose/changes
There's an issue where the
kbase_session_backupcookie seems to be blocked or disappears on occasion. We're not entirely sure what's causing this right now - it could be some browser policy changes with regard to how the cookie is created. What it means is that direct HTTP communication to KBase services via iframes (i.e. report view and downloads) can fail by complaining that the user isn't logged in. This forces a new backup cookie to be created on page reload.Unit and integration tests pending, but I wanted an image made to get in narrative-dev. That and production are the only environments that'll fail this way.
Jira Ticket / Issue
Related Jira ticket: https://kbase-jira.atlassian.net/browse/PTV-1913
DATAUP-69 Adds a PR template)Testing Instructions
Dev Checklist:
formatandcheckon changed Python code manually or with a git precommit hookUpdating Version and Release Notes (if applicable)