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.

Sign up for GitHub

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

Jump to bottom

Import and Export Modularized #14469

Merged
merged 4 commits into from
Aug 1, 2018
Merged

Import and Export Modularized #14469

merged 4 commits into from
Aug 1, 2018

Conversation

Piyush3079
Copy link
Contributor

This pr contains the modular code for Import and Export js files.

This pr is in continuation of Mod_Jd_Navigation. Before reviewing this pr, review Mod_Js_Navigation and merge and then compare with this pr.
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

@devenbansod devenbansod self-requested a review July 30, 2018 10:35
@devenbansod devenbansod self-assigned this Jul 30, 2018
@devenbansod devenbansod changed the base branch from master to gsoc-js-refactoring July 30, 2018 10:35
@devenbansod
Copy link
Member

Please rebase and remove conflicts.

@codecov
Copy link

codecov bot commented Jul 30, 2018 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (gsoc-js-refactoring@00e9049). Click here to learn what that means.
The diff coverage is 15.17%.

@@                  Coverage Diff                   @@
##             gsoc-js-refactoring   #14469   +/-   ##
======================================================
  Coverage                       ?   49.93%           
  Complexity                     ?    14435           
======================================================
  Files                          ?      504           
  Lines                          ?    68973           
  Branches                       ?        0           
======================================================
  Hits                           ?    34442           
  Misses                         ?    34531           
  Partials                       ?        0

@Piyush3079 Piyush3079 force-pushed the Mod_Js_Import_Export branch 6 times, most recently from 3b774a6 to f6f397d Compare July 30, 2018 14:07
Piyush3079
Piyush3079 commented Jul 30, 2018
View reviewed changes
Copy link
Contributor Author

@Piyush3079 Piyush3079 left a comment

Choose a reason for hiding this comment

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

General comments to increase the readability of the pr.

.eslintignore
@@ -1,3 +1,5 @@
js/vendor/
tmp/
vendor/
js/dist/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add some folders to avoid check of linting

.gitignore
@@ -56,3 +56,6 @@ composer.lock
# NPM
/node_modules/
yarn.lock
# Javascript Bundle
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add some folders to gitignore to avoid pushing them to the repo

js/src/classes/Chart.js
@@ -0,0 +1,678 @@
/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Chart class needed for drawing chart of server_status_queries.php page

js/src/consts/files.js
server_status_advisor: ['server_status_advisor'],
server_status_processes: ['server_status_processes'],
server_status_variables: ['server_status_variables'],
user_password: ['server_privileges']
user_password: ['server_privileges'],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Other server related files and server import and export files are added to the object

js/src/export.js
@@ -0,0 +1,391 @@
/* vim: set expandtab sw=4 ts=4 sts=4: */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modular code for exporting sql.
Functions required for this file are imported from a files js/src/functions/export.js

server_import.php
@@ -20,7 +20,7 @@
$response = Response::getInstance();
$header = $response->getHeader();
$scripts = $header->getScripts();
$scripts->addFile('import.js');
$scripts->addFile('import');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

.js extension removed from file to enable dynamic import

server_replication.php
@@ -26,7 +26,7 @@
$header = $response->getHeader();
$scripts = $header->getScripts();
$scripts->addFile('server_privileges');
$scripts->addFile('replication.js');
$scripts->addFile('replication');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

.js extension removed from file to enable dynamic import

server_status_queries.php
@@ -30,8 +30,8 @@
$scripts->addFile('vendor/jqplot/plugins/jqplot.highlighter.js');
$scripts->addFile('vendor/jqplot/plugins/jqplot.enhancedPieLegendRenderer.js');
$scripts->addFile('vendor/jquery/jquery.tablesorter.js');
$scripts->addFile('server_status_sorter.js');
$scripts->addFile('server_status_queries.js');
$scripts->addFile('server_status_sorter');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

.js extension removed from file to enable dynamic import

server_user_groups.php
@@ -23,7 +23,7 @@
$response = Response::getInstance();
$header = $response->getHeader();
$scripts = $header->getScripts();
$scripts->addFile('server_user_groups.js');
$scripts->addFile('server_user_groups');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

.js extension removed from file to enable dynamic import

templates/display/export/options_format.twig
@@ -19,6 +19,6 @@
{#- If the time limit set is zero, then time out won't occur so no need
to check for time out. -#}
{%- if exec_time_limit > 0 %}
onclick="check_time_out({{ exec_time_limit }})"
data-timeout={{ exec_time_limit }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed inline onClick event which requires the function to be available in the global scope.
Added the value of timeOut in data and the add an event in export.js to make call to this function.

@Piyush3079
Edit ndex files and more files whitelisted in ajax.js and files.js for
f92e1eb 
adding them in modular code.
Signed-off-by: Piyush Vijay <piyushvijay.1997@gmail.com>
@Piyush3079
Modularize server related remaining files like replication, plugins, …
a99acfe 
…queries, processes.

Sever monitor is also modularised and is not working but jQplot is not imported as it is giving error with webpack.
Another issue with these files is that table sorter is also not woking
working properly as it is not available on npm as plugin.
Signed-off-by: Piyush Vijay <piyushvijay.1997@gmail.com>
@Piyush3079
Remove some functions to make this pr reproduceable on local machine.
85c16c1 
Signed-Off-By: Piyush Vijay <piyushvijay.1997@gmail.com>
@Piyush3079
Add event for submit of export sql form to remove error of undefined …
d111a2c 
…function.

Signed-Off-By: Piyush Vijay <piyushvijay.1997@gmail.com>
devenbansod
devenbansod approved these changes Aug 1, 2018
View reviewed changes
Copy link
Member

@devenbansod devenbansod left a comment

Choose a reason for hiding this comment

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

Tested locally. Works fine for me.

@devenbansod devenbansod merged commit 5e9d705 into phpmyadmin:gsoc-js-refactoring Aug 1, 2018
@Piyush3079
Copy link
Contributor Author

With this pr the tablesorter plugin of jquery is broken as the plugin available on npm is comparatively large as compared to the one we are using with the downloaded version. This issue is resolved in the future pr as these plugins are downloaded from github with the versions upgraded so that the plugins will not give error with webpack based code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@devenbansod devenbansod devenbansod approved these changes

Assignees

@devenbansod devenbansod

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Piyush3079 @devenbansod