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

Clear 1.7.0.0.sql #6692

Merged
merged 1 commit into from Oct 19, 2016

Conversation

Projects
None yet
6 participants
@martinfojtik
Contributor

martinfojtik commented Oct 14, 2016

Questions Answers
Branch? develop
Description? merge some queries
Type? improvement
Category? CO
BC breaks? no
Deprecations? no
Fixed ticket?
How to test? update prestashop or use a sql validator

@aleeks aleeks removed the Code ✔️ label Oct 17, 2016

@aleeks

aleeks suggested changes Oct 17, 2016 edited

UPDATEPREFIX_quick_accessSETlink= "product/new" WHERElink= "index.php?controller=AdminProducts&addproduct";

need to be changed into :

UPDATEPREFIX_quick_accessSETlink= "index.php/product/new" WHERElink= "index.php?controller=AdminProducts&addproduct";

It's an error, (not you), see install-dev/data/xml/quick_access.xml

After this change, I make a test with the update file! 😄

Show outdated Hide outdated install-dev/upgrade/sql/1.7.0.0.sql Outdated
Show outdated Hide outdated install-dev/upgrade/sql/1.7.0.0.sql Outdated
@aleeks

aleeks approved these changes Oct 17, 2016

@aleeks

This comment has been minimized.

Show comment
Hide comment
@aleeks

aleeks Oct 17, 2016

Contributor

Hello @martinfojtik
It's ok for me, I'm waiting travis test, and I make a test from 1.6.1.7!
Very good job !

Contributor

aleeks commented Oct 17, 2016

Hello @martinfojtik
It's ok for me, I'm waiting travis test, and I make a test from 1.6.1.7!
Very good job !

@aleeks aleeks dismissed a stale review Oct 18, 2016

I make a other clean review..

@aleeks

aleeks suggested changes Oct 18, 2016 edited

Hello, can you fix this little things please ?
I was testing this sql file and I had some mistake!

If i'm not clear, say me!

Thank you for your job, really.

Show outdated Hide outdated install-dev/upgrade/sql/1.7.0.0.sql Outdated
Show outdated Hide outdated install-dev/upgrade/sql/1.7.0.0.sql Outdated
Show outdated Hide outdated install-dev/upgrade/sql/1.7.0.0.sql Outdated
@@ -64,7 +64,7 @@ ALTER TABLE `PREFIX_product_shop` ADD `show_condition` TINYINT(1) NOT NULL DEFAU
/* PHP:ps_1700_add_payment_preferences_tab(); */;

This comment has been minimized.

@aleeks

aleeks Oct 18, 2016

Contributor

Line 62, same as last_connection_date, can you add :

ALTER TABLE PREFIX_product CHANGE available_date available_date DATE NULL DEFAULT NULL;
ALTER TABLE PREFIX_product_shop CHANGE available_date available_date DATE NULL DEFAULT NULL;

and report it on db_structure.sql lines : 1433 & 1478

sql3

@aleeks

aleeks Oct 18, 2016

Contributor

Line 62, same as last_connection_date, can you add :

ALTER TABLE PREFIX_product CHANGE available_date available_date DATE NULL DEFAULT NULL;
ALTER TABLE PREFIX_product_shop CHANGE available_date available_date DATE NULL DEFAULT NULL;

and report it on db_structure.sql lines : 1433 & 1478

sql3

@@ -81,40 +81,32 @@ ALTER TABLE `PREFIX_product_lang` ADD `social_sharing_description` VARCHAR( 25

This comment has been minimized.

@aleeks

aleeks Oct 18, 2016

Contributor

Line 76 (i can point this), same as last_connection_date :

ALTER TABLE PREFIX_product_attribute_shop CHANGE available_date available_date DATE NULL DEFAULT NULL;
ALTER TABLE PREFIX_product_attribute CHANGE available_date available_date DATE NULL DEFAULT NULL;

and report it on db_structure.sql lines : 1512 & 1532

@aleeks

aleeks Oct 18, 2016

Contributor

Line 76 (i can point this), same as last_connection_date :

ALTER TABLE PREFIX_product_attribute_shop CHANGE available_date available_date DATE NULL DEFAULT NULL;
ALTER TABLE PREFIX_product_attribute CHANGE available_date available_date DATE NULL DEFAULT NULL;

and report it on db_structure.sql lines : 1512 & 1532

Show outdated Hide outdated install-dev/upgrade/sql/1.7.0.0.sql Outdated
@jocel1

Would be interesting to merge this PR after merging #6531 .
I think the issues reported by @aleeks are fixed by PR #6531 (a SET SESSION sql_mode='' present in DbPDO.php but not in DbMySQLi.php)

@julienbourdeau

This comment has been minimized.

Show comment
Hide comment
@julienbourdeau

julienbourdeau Oct 19, 2016

Contributor

Great job @martinfojtik! It will make our lives much easier :)

Thank you very much @aleeks for testing and reviewing in details.

Contributor

julienbourdeau commented Oct 19, 2016

Great job @martinfojtik! It will make our lives much easier :)

Thank you very much @aleeks for testing and reviewing in details.

@aleeks

aleeks approved these changes Oct 19, 2016

@aleeks aleeks added the WIP label Oct 19, 2016

@aleeks

This comment has been minimized.

Show comment
Hide comment
@aleeks

aleeks Oct 19, 2016

Contributor

Executed without error !
Really good job @martinfojtik !!

I put this PR in WIP in order to not merge it, waiting for #6531!

@jocel1 agree to test your upgrade one time with this script ?

Contributor

aleeks commented Oct 19, 2016

Executed without error !
Really good job @martinfojtik !!

I put this PR in WIP in order to not merge it, waiting for #6531!

@jocel1 agree to test your upgrade one time with this script ?

@julienbourdeau

This comment has been minimized.

Show comment
Hide comment
@julienbourdeau

julienbourdeau Oct 19, 2016

Contributor

I think it will be easier to rebase @jocel1's PR against this one. Let's merge it!

Contributor

julienbourdeau commented Oct 19, 2016

I think it will be easier to rebase @jocel1's PR against this one. Let's merge it!

@julienbourdeau julienbourdeau merged commit 0a09bd0 into PrestaShop:develop Oct 19, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@aleeks aleeks removed the WIP label Oct 19, 2016

@martinfojtik

This comment has been minimized.

Show comment
Hide comment
@martinfojtik

martinfojtik Oct 19, 2016

Contributor

Thanks @aleeks for testing my changes 👍

Contributor

martinfojtik commented Oct 19, 2016

Thanks @aleeks for testing my changes 👍

@martinfojtik martinfojtik deleted the martinfojtik:patch-26 branch Oct 19, 2016

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