-
Notifications
You must be signed in to change notification settings - Fork 754
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
Moved ImportExport cookie consent settings to correct project #2431
Conversation
@valadas would you mind to check in your install, whether importexportsettings table has been created using proper oq, thank you (I doubt this was caused by using the script). |
@sleupold when I initially opened the issue, I tought it was because of the object qualifier, but after investigating, it was also the case without an OQ. So I digged deeper and it is because the original script (that was part of the site scripts) was trying to modify a column on a table that does not exist yet. That table is created by the ExportImport module and module installation runs after the site scripts. So I moved that into the module sql scripts so it runs after the module is installed. |
@valadas |
Maybe... I suggest merging this for now because it solves the Dnn install issue and it follows how it is implemented now (as a module) and you could open a new issue to move the whole module into the API. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the current environment, this seems to be correct. However, long-term this is a situation where the "home" for things is unclear.
@donker since this is an item you're actively working on, can you review/approve and merge? |
I was unsure where this should go, so good catch Daniel. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was unsure where it needed to go. This is better.
Closes #2430
Summary
Moved ImportExport cookie consent settings sql script into correct project.
In #2369 an sql script was added to modify the import/export settings in the sql table, but that script being in the Dnn installation and not on the module scripts caused a clean Dnn install to work due to the fact that all Dnn install scripts run first and then the modules install. So the old way would have only worked on upgrades because that column would already exist.
On new installations, it was trying to update a column that did not already exist.
This pull request brings that script at the correct place an allows for a clean install of Dnn to work.