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

[5.1] Update TourTable.php to allow save on tours #43031

Merged
merged 5 commits into from
Mar 17, 2024

Conversation

obuisard
Copy link
Contributor

@obuisard obuisard commented Mar 14, 2024

Summary of Changes

Since the introduction of the welcome tour, the addition of the autostart column fails the saving of tours.

Testing Instructions

Go to System -> Guided Tours, create a tour and save.
Try also 'duplicate' and 'save as copy'.

Actual result BEFORE applying this Pull Request

Error 'Save failed with the following error: Column 'autostart' cannot be null'.

Expected result AFTER applying this Pull Request

No error on save.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

Column autostart set to 0 if null
@obuisard obuisard changed the title Update TourTable.php [5.1] Update TourTable.php to allow save on tours Mar 14, 2024
@brianteeman
Copy link
Contributor

would it not be better to update the sql instead?

@obuisard
Copy link
Contributor Author

obuisard commented Mar 14, 2024

would it not be better to update the sql instead?

The database already defaults autostart to 0.
At this point, the autostart is not present as an option to the user. Once we have the full functionality (we will make auto-start as an option to the tour in the form), we can remove that additional test.

@brianteeman
Copy link
Contributor

yes but you could also set it to allow null couldnt you and then you dont need this temporary code?

@obuisard
Copy link
Contributor Author

yes but you could also set it to allow null couldnt you and then you dont need this temporary code?

Oh I see what you mean... It felt 'safer' this way so I don't mess with the database again and add extra files.

@garstud
Copy link

garstud commented Mar 15, 2024

I have tested this item ✅ successfully on cf6cbc2


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43031.

@Quy Quy added the bug label Mar 15, 2024
@Quy
Copy link
Contributor

Quy commented Mar 16, 2024

I have tested this item ✅ successfully on 1bef0eb


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43031.

1 similar comment
@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on 1bef0eb


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43031.

@Quy
Copy link
Contributor

Quy commented Mar 16, 2024

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43031.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 16, 2024
@bembelimen bembelimen merged commit 3f6ecdd into joomla:5.1-dev Mar 17, 2024
4 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 17, 2024
@bembelimen
Copy link
Contributor

Thx

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

Successfully merging this pull request may close these issues.

7 participants