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.

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

Fixed issue #19262: Invalid fruity_twentythree template directory on … #3642

Merged

Conversation

olleharstedt
Copy link
Collaborator

…PostgreSQL

Fixed issue #:
New feature #:
Dev:

Copy link
Collaborator

@Shnoulle Shnoulle left a comment

Choose a reason for hiding this comment

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

Need testing

How get multiple theme, i need to add it manually ?

@olleharstedt
Copy link
Collaborator Author

Need testing

How get multiple theme, i need to add it manually ?

A fresh install from dbversion 615 or 616 is enough, using Postgres. Then updating to 617 will clear all settings, without this fix. Again, using Postgres.

@Shnoulle
Copy link
Collaborator

A fresh install from dbversion 615 or 616 is enough, using Postgres. Then updating to 617 will clear all settings, without this fix. Again, using Postgres.

But : i don't have multiple theme lines, then i can not control if the update fix the DB

@olleharstedt
Copy link
Collaborator Author

A fresh install from dbversion 615 or 616 is enough, using Postgres. Then updating to 617 will clear all settings, without this fix. Again, using Postgres.

But : i don't have multiple theme lines, then i can not control if the update fix the DB

You don't have entries in table lime_template_configuration after a fresh install?

@Shnoulle Shnoulle added Code review done Version checked for code issue without testing and removed Needs code review labels Nov 29, 2023
@Shnoulle
Copy link
Collaborator

A fresh install from dbversion 615 or 616 is enough, using Postgres. Then updating to 617 will clear all settings, without this fix. Again, using Postgres.

But : i don't have multiple theme lines, then i can not control if the update fix the DB

You don't have entries in table lime_template_configuration after a fresh install?

I have : but not multiple :)

@Shnoulle
Copy link
Collaborator

Shnoulle commented Nov 29, 2023

Need to test if deleteDuplicateTemplateConfigurationEntries really delete NOT used TemplateConfiguration
(i think find get the 1st id and not the last)

@Shnoulle
Copy link
Collaborator

Tested : seems there are an issue with the original commit …

Seems it must keep the minimal id, not the max.
I try again

@Shnoulle
Copy link
Collaborator

Shnoulle commented Nov 29, 2023

Yep : confirm the issue in case of multiple theme

  1. get a 216 DB version
  2. Create a survey and set a theme
  3. Clone the theme line in DB
  4. Update an option in theme option, and save
  5. Update to this commit
  6. You loose update done in #4

find get the 1st element, not the last.

Else : this commit is OK in pgsql : keep one configuration only (but the bad one)

@olleharstedt
Copy link
Collaborator Author

@Shnoulle did you do both code-review and test?

@Shnoulle
Copy link
Collaborator

@Shnoulle did you do both code-review and test?

I test : but i can not do both …

I have to report the issue about them option loose, but i don't know how to get it except do it in DB (and it's not related to pgsql)

@Shnoulle
Copy link
Collaborator

Test way and result : #3642 (comment)

@tiborpacalat tiborpacalat added Tested OK This PR has been tested by QA and works as expected and removed Needs testing labels Dec 11, 2023
@tiborpacalat tiborpacalat merged commit 7558178 into master Dec 11, 2023
20 checks passed
@tiborpacalat tiborpacalat deleted the bugs/19262-invalid-fruity_twentythree-template-directory branch December 11, 2023 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code review done Version checked for code issue without testing Tested OK This PR has been tested by QA and works as expected
Projects
None yet
3 participants