Stop passing sessionid cookie in to media uploader #29489
Conversation
it has not served a purpose in years and is a poor practice. Also remove references to swfURL, which was removed 5 years ago.
| sessionid: initial_page_data('sessionid'), | ||
| })); | ||
| print_uploader.options, | ||
| ); |
There was a problem hiding this comment.
What am I missing. Our .eslintrc.js requires comma-dangle, and Parsing error isn't what I'd expect in that situation anyway, so it can't be that. Am I like counting parentheses wrong?
There was a problem hiding this comment.
comma-dangle is for objects & arrays, but not parameter lists in function calls. This will also break deploy because the uglifier will interpret it as a syntax error.
Today I learned that the trailing comma is allowed in function calls as of ECMAScript 2017, but we're not there yet.
There was a problem hiding this comment.
Ah, okay thank you sharing that. I assumed because of https://eslint.org/docs/rules/comma-dangle#functions that comma-dangle included function arguments. But maybe this parsing error is coming from something other than eslint 🤔
There was a problem hiding this comment.
I think eslint has logic so that function arguments aren't included in the comma-dangle rule, even when it's set to "always", unless you also have an eslint environment that supports that syntax, like by setting ecmaVersion to 8+
There was a problem hiding this comment.
Ah okay gotcha. That was the piece I was missing. Thanks!
| swfURL: initial_page_data("swfURL"), | ||
| }, uploader.options)); | ||
| uploader.options, | ||
| ); |
dannyroberts
added
product/invisible
dannyroberts
deleted the
dmr/remove-vestigial-reference-to-session-cookie
branch
it has not served a purpose in years and is a poor practice.
Also remove references to swfURL, which was removed 5 years ago.
Summary
https://dimagi-dev.atlassian.net/browse/SAAS-11746
Corresponding PRs dimagi/MediaUploader#29 and dimagi/Vellum#1004 are part of the same cleanup, but I believe each can actually be merged separately without causing errors from mismatched versions. (That's how little used the values are.)
Feature Flag
Product Description
None
Safety Assurance
Automated test coverage
QA Plan
Not planning QA
Safety story
I tested (1) bulk media upload (2) case list media upload in app builder (3) question media upload in form builder, both locally and on staging. On staging this was tested without the other two related branches to make sure it's safe to deploy on its own, allowing us to later deploy the others as no-op cleanups rather than wondering whether they need to be perfectly timed.
Rollback instructions