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

Variables: Fixes maximum call stack bug for empty value #25503

Merged
merged 1 commit into from Jun 15, 2020

Conversation

hugohaggmark
Copy link
Contributor

What this PR does / why we need it:
When using an empty value for a Text variable the logic that checked for variable changes in URL got into an endless loop. This PR tries to remedy this.

Which issue(s) this PR fixes:
Fixes #25485

Special notes for your reviewer:

@hugohaggmark hugohaggmark added this to the 7.1 milestone Jun 10, 2020
@hugohaggmark hugohaggmark requested review from torkelo, aocenas and a team June 10, 2020 08:19
@hugohaggmark hugohaggmark self-assigned this Jun 10, 2020
@hugohaggmark hugohaggmark requested review from tskarhed and removed request for a team June 10, 2020 08:19
@aocenas
Copy link
Member

aocenas commented Jun 10, 2020

Not sure how related this is but seem like the variable is not preserved in the url. If I save the dashboard and then open it in separate tab it shows the variable filled in but it isn't in the url and it seems to start working only after I click into the input and out.

Screenshot from 2020-06-10 14-14-32

@hugohaggmark
Copy link
Contributor Author

Not sure how related this is but seem like the variable is not preserved in the url. If I save the dashboard and then open it in separate tab it shows the variable filled in but it isn't in the url and it seems to start working only after I click into the input and out.

Screenshot from 2020-06-10 14-14-32

good catch, will look into this!

@hugohaggmark
Copy link
Contributor Author

Not sure how related this is but seem like the variable is not preserved in the url. If I save the dashboard and then open it in separate tab it shows the variable filled in but it isn't in the url and it seems to start working only after I click into the input and out.

Screenshot from 2020-06-10 14-14-32

If I understand your comment right @aocenas this is the intended behavior for all variables.

@torkelo torkelo added this to In Review in Frontend Platform Backlog Jun 11, 2020
@hugohaggmark
Copy link
Contributor Author

Is there some additional work that is needed before approval here @aocenas @tskarhed

Copy link
Contributor

@tskarhed tskarhed left a comment

Choose a reason for hiding this comment

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

Looks good! Error is no longer there!

@hugohaggmark hugohaggmark merged commit bd44d97 into master Jun 15, 2020
Frontend Platform Backlog automation moved this from In Review to Done Jun 15, 2020
@hugohaggmark hugohaggmark deleted the hugoh/textvariable-fix branch June 15, 2020 09:49
@aocenas
Copy link
Member

aocenas commented Jun 15, 2020

@hugohaggmark
So there was still some issues based on the conversations in slack mainly:

But you should not come to the state when you see query: some text while the actual value is something else.

This still seems like a bug to me.

@aocenas
Copy link
Member

aocenas commented Jun 15, 2020

But I am ok if that is solved separately.

@hugohaggmark
Copy link
Contributor Author

Would be great if you could file that as a separate issue @aocenas 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Variables: Text input variable does not work because "Maximum call stack size exceeded"
3 participants