Skip to content
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

Improve export copy & logic #6023

Merged
merged 3 commits into from
Dec 6, 2023
Merged

Conversation

ClementPasteau
Copy link
Collaborator

@ClementPasteau ClementPasteau commented Dec 6, 2023

image

componentWillMount() {
// Fetch limits when the export launcher is opened, to ensure we display the
// latest limits.
this.props.authenticatedUser.onSubscriptionUpdated();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a case where the call to limits gave me the old value after the build finished (probably some latency on the backend side) and I realised I had not way of getting the right value, apart from going to my profile to retrigger a refresh. (I think it's fine to make this call on every mount?)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably ok, even if a bit "too much" for what is needed.
My only concern is the name "onSubscriptionUpdated" => huh.
I would rather have a "onRefreshUserLimits" (even if the implementation is the same, well, at least the component is expressing its need - it's then the responsibility of the authenticatedUser to try to minimize work as much as it can)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to only fetch limits 👍
We indeed don't need sub or usages info (I updated other part of the codebase where we fetch too much too)

@ClementPasteau ClementPasteau merged commit 1b4c5c1 into master Dec 6, 2023
6 checks passed
@ClementPasteau ClementPasteau deleted the fetch-limits-after-build branch December 6, 2023 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants