-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Bypass cache/dirty when canceling transaction #7503
Bypass cache/dirty when canceling transaction #7503
Conversation
@@ -32,7 +32,7 @@ | |||
class="c-swatch" | |||
:style="{ background: options.value }" | |||
role="img" | |||
:aria-label="None" | |||
aria-label="None" |
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.
Driveby as it was throwing a warning every time the toolbar was brought up.
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.
Nice catch
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7503 +/- ##
=======================================
Coverage 55.39% 55.39%
=======================================
Files 671 671
Lines 27010 27010
Branches 2631 2631
=======================================
Hits 14961 14961
- Misses 11326 11328 +2
+ Partials 723 721 -2
*This pull request uses carry forward flags. Click here to find out more.
... and 10 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
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.
This all LGTM. Nice work!
@@ -35,7 +35,7 @@ const defaultFrameBorderColor = '#e6b8af'; //default border color | |||
const defaultBorderTargetColor = '#acacac'; | |||
const defaultTextColor = '#acacac'; // default text color | |||
const inheritedColor = '#acacac'; // inherited from the body style | |||
const pukeGreen = '#6aa84f'; //Ugliest green known to man | |||
const pukeGreen = '#6aa84f'; //Ugliest green known to man 🤮 |
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.
🤢
@@ -411,4 +411,39 @@ test.describe('Flexible Layout styling', () => { | |||
page.getByLabel('StackedPlot1 Frame').getByLabel('Stacked Plot Style Target') | |||
); | |||
}); | |||
|
|||
test('Styling, and then canceling reverts to previous style', async ({ page }) => { |
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.
fantastic
@@ -75,10 +75,10 @@ export default class Transaction { | |||
|
|||
_clear() { | |||
const promiseArray = []; | |||
const refresh = this.objectAPI.refresh.bind(this.objectAPI); | |||
const action = (obj) => this.objectAPI.refresh(obj, true); |
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.
we love our arrow functions don't we folks? nice change
* @returns {Promise} the provided object, updated to reflect the latest persisted state of the object. | ||
*/ | ||
async refresh(domainObject) { | ||
const refreshedObject = await this.get(domainObject.identifier); | ||
async refresh(domainObject, forceRemote = false) { |
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.
nice! the forceRemote
option on get()
came in handy @jvigliotta
Closes #7233
Describe your changes:
When canceling a transaction, we call
refresh
on the object, this in turn callsget
, which by default can use a cached/dirty version of the object. This isn't very helpful as the cached object is the dirty object we're trying revert! To fix this, we instead useforceRemote
on theget
to get the untouched version of the object.All Submissions:
Author Checklist
type:
label? Note: this is not necessarily the same as the original issue.Reviewer Checklist