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

Restore SQL query after session expire #14046

Merged
merged 3 commits into from Mar 16, 2018

Conversation

Projects
None yet
3 participants
@shucon
Contributor

shucon commented Feb 28, 2018

Partially Fixes: #13654

Signed-off-by: Saksham Gupta shucon01@gmail.com

Before submitting pull request, please check that every commit:

  • Has proper Signed-Off-By
  • Has commit message which describes it
  • Is needed on it's own, if you have just minor fixes to previous commits, you can squash them
  • Any new functionality is covered by tests

shucon added some commits Feb 28, 2018

Restore SQL query after session expire
Partially Fixes: #13654

Signed-off-by: Saksham Gupta <shucon01@gmail.com>
Fix whitespaces
Signed-off-by: Saksham Gupta <shucon01@gmail.com>
@phpmyadmin-bot

This comment has been minimized.

phpmyadmin-bot commented on e5ff7f3 Feb 28, 2018

This commit contains trailing whitespace, what is prohibited in phpMyAdmin. Please check our Developer guidelines for more information.

Offending files: js/sql.js

@shucon

This comment has been minimized.

Contributor

shucon commented Feb 28, 2018

Once this is approved I'll try to do something similar for Insert page.

@nijel

It should handle values stored in cookies as well, preferably reusing existing code.

js/sql.js Outdated
/**
* Restores SQL query after timeout login
*/
codemirror_editor.setValue(window.localStorage.auto_saved_sql);

This comment has been minimized.

@nijel

nijel Mar 1, 2018

Member

There is already code to handle this:

phpmyadmin/js/functions.js

Lines 1180 to 1187 in 733a3f1

} else if (queryType === 'saved') {
if (isStorageSupported('localStorage') && typeof window.localStorage.auto_saved_sql !== 'undefined') {
setQuery(window.localStorage.auto_saved_sql);
} else if (Cookies.get('auto_saved_sql')) {
setQuery(Cookies.get('auto_saved_sql'));
} else {
PMA_ajaxShowMessage(PMA_messages.strNoAutoSavedQuery);
}

So it should load it from cookies as well (insertQuery('saved') would do it, but it would also error on no saved query what is not desired behavior here).

Add cookie restore and empty save error
Signed-off-by: Saksham Gupta <shucon01@gmail.com>
@codecov

This comment has been minimized.

codecov bot commented Mar 2, 2018

Codecov Report

Merging #14046 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master   #14046      +/-   ##
============================================
+ Coverage     55.43%   55.43%   +<.01%     
  Complexity    14364    14364              
============================================
  Files           494      494              
  Lines         70544    70544              
============================================
+ Hits          39103    39105       +2     
+ Misses        31441    31439       -2
@shucon

This comment has been minimized.

Contributor

shucon commented Mar 10, 2018

@nijel I think it's ok now, can you please review it?

if (isStorageSupported('localStorage')) {
window.localStorage.auto_saved_sql = query;
} else {
Cookies.set('auto_saved_sql', query);

This comment has been minimized.

@nijel

nijel Mar 16, 2018

Member

Should it really save the empty query here?

This comment has been minimized.

@shucon

shucon Mar 16, 2018

Contributor

Yes, because if the query is empty then on restore the last character was saved and got restored, that wasn't looking good.

@nijel nijel merged commit 5a1ceef into phpmyadmin:master Mar 16, 2018

3 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
Codacy/PR Quality Review Good work! A positive pull request.
Details
DCO All commits have a DCO sign-off from the author
Scrutinizer Analysis: No new issues – Tests: passed
Details
@nijel

This comment has been minimized.

Member

nijel commented Mar 16, 2018

Merged, thanks for your contribution!

@nijel nijel added this to the 4.8.0 milestone Mar 16, 2018

@nijel nijel self-assigned this Mar 16, 2018

@shucon shucon deleted the shucon:login branch Mar 18, 2018

mauriciofauth added a commit that referenced this pull request Apr 6, 2018

Revert "Merge pull request #14046 from shucon/login"
This reverts commit 5a1ceef, reversing
changes made to 063f380.

Signed-off-by: Maurício Meneghini Fauth <mauriciofauth@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment