Update missing SQL compare to a fresh install #689
Conversation
M0rgan01
commented
Apr 16, 2024
•
M0rgan01
commented
| Questions | Answers |
|---|---|
| Description? | Update missing SQL compare to a fresh install |
| Type? | improvement |
| BC breaks? | no |
| Deprecations? | no |
| Fixed ticket? | - |
| Sponsor company | - |
| How to test? | Use https://github.com/PrestaShop/SeamlessUpgradeToolbox/tree/main/autoupgrade-scripts for test versions concerned |
M0rgan01
changed the title
M0rgan01
force-pushed
the
fix/sql
branch
12 times, most recently
from
0ac2cfb to
c6ab9e5
Compare
upgrade/sql/8.1.0.sql
Outdated
| @@ -58,6 +58,13 @@ ALTER TABLE `PREFIX_product_shop` MODIFY COLUMN `redirect_type` ENUM( | |||
| UPDATE `PREFIX_product` SET `redirect_type` = 'default' WHERE `redirect_type` = '404' OR `redirect_type` = '' OR `redirect_type` IS NULL; | |||
| UPDATE `PREFIX_product_shop` SET `redirect_type` = 'default' WHERE `redirect_type` = '404' OR `redirect_type` = '' OR `redirect_type` IS NULL; | |||
|
|
|||
| ALTER TABLE `PREFIX_product` MODIFY COLUMN `redirect_type` ENUM( | |||
| '404','410','301-product','302-product','301-category','302-category','200-displayed','404-displayed','410-displayed','default' | |||
There was a problem hiding this comment.
Already present few lines above, you can modify the original one ;-)
There was a problem hiding this comment.
Thanks for the review :) I think we need to do this in 3 steps:
- Add the ENUMS while keeping the
''so as not to lose the existing values. - Update to
defaultwhere''and ... - Remove the unnecessary
''
There was a problem hiding this comment.
Yes, exactly as you say. :-)
I kept the empty string in the enum, because products created pre 1.7.8.0 had it by default - https://github.com/PrestaShop/PrestaShop/blob/aab3f53c9c31d79a6c60d9744d2a175c4d6c5194/install-dev/data/db_structure.sql#L1647
And also, it was implemented in 8.1.0, so allowing that empty string was to avoid a BC break.
There was a problem hiding this comment.
Soooo, actually, I think we should do this in 9.0.0 and keep this like it is. 🤔
There was a problem hiding this comment.
Ok, well seen !
I changed the updating of data and columns in the v9 script, this way we catch up with potential insertions with empty string
M0rgan01
force-pushed
the
fix/sql
branch
14 times, most recently
from
e766f12 to
54b2b26
Compare
M0rgan01
force-pushed
the
fix/sql
branch
2 times, most recently
from
c7aff52 to
2affe63
Compare
|
@kpodemski Are you saying that because the merge and release of the module will come after the 8.1.6 release? |
|
@M0rgan01 I think we won't be able to release 8.1.6 with autoupgrade |
|
@M0rgan01 Can you put the things to |
|
@Hlavtox The idea is to have a specific catch-up file, to separate the "real" migrations from those that have been forgotten. |
|
@kpodemski I updated the migration file to 8.1.7, is that good for you? |
There was a problem hiding this comment.
Hi @M0rgan01
Thank you for your PR, I tested it :
from 1.7.0.1 to :
1.7.1.0
1.7.2.0
1.7.3.0
1.7.4.0
1.7.5.0 -> impossible to upgrade
1.7.6.0
1.7.7.0
1.7.8.0
1.7.8.11
from 1.7.5.0 to :
8.0.0
8.1.0
from 8.0.5 to 9.0.0
Because all seems to works as expected, It's QA ✔️
Thank you
