RTC Language Change PR-5.2-dev Guided Tours Pending

User tests: Successful: Unsuccessful:

avatar obuisard
obuisard
17 Sep 2024

Pull Request for Issue #44094.

Summary of Changes

The 'Auto Start' was added to the tour parameters.
The tour needs to take into account that parameter in adding a new step in the tour.

Testing Instructions

You need to test on a fresh install and on update.
Run the tour 'How to create a tour?'.

Actual result BEFORE applying this Pull Request

The tour runs and bypasses the parameter 'Auto Start'.

Expected result AFTER applying this Pull Request

The tour runs and has a step for the 'Auto Start' parameter, just before the step 'Save and close'.

autostart

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

avatar obuisard obuisard - open - 17 Sep 2024
avatar obuisard obuisard - change - 17 Sep 2024
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 17 Sep 2024
Category SQL Administration com_admin Postgresql Language & Strings Installation
avatar obuisard
obuisard - comment - 17 Sep 2024

Richard @richard67 I am not sure about the updated scripts.

I cannot add a step in a tour between other steps and ensure the steps are in the right order.
Therefore I deleted all steps for the tour, then re-introduced them.
I don't see another way to do this.

Also, I am not sure it's good practice to use UNION ALL in the INSERT INTO ... SELECT.
So I have done without in mySQL and with in PostgreSQL, not sure which is best, if any.

avatar obuisard obuisard - change - 17 Sep 2024
The description was changed
avatar obuisard obuisard - edited - 17 Sep 2024
avatar obuisard
obuisard - comment - 17 Sep 2024

Brian @brianteeman How is the wording for you?

avatar obuisard obuisard - change - 17 Sep 2024
Title
[5.2] [Guided Tours] Addition of 'Auto start' parameter in 'How to create a tour?' tour
[5.2] [Guided Tours] Addition of 'Auto start' step in 'How to create a tour?' tour
avatar obuisard obuisard - edited - 17 Sep 2024
avatar obuisard obuisard - change - 17 Sep 2024
Labels Added: Language Change PR-5.2-dev Guided Tours
avatar richard67
richard67 - comment - 18 Sep 2024

Richard @richard67 I am not sure about the updated scripts.

@obuisard I can’t check before weekend.

avatar richard67
richard67 - comment - 19 Sep 2024

Richard @richard67 I am not sure about the updated scripts.

I cannot add a step in a tour between other steps and ensure the steps are in the right order. Therefore I deleted all steps for the tour, then re-introduced them. I don't see another way to do this.

@obuisard Does that mean the tour steps need to be saved in database in the same order as they appear in the list because they obviously don't have an order column? As far as I can see that's the case, and if so, then indeed there is no way to insert a record somewhere in the middle. But to be honest: That's poor design. It always should be possible to add a step in the middle, and the right way to do that would be to use an ordering column so that ou always can reorder steps or add steps in the middle.

avatar obuisard
obuisard - comment - 19 Sep 2024

That's why I am reaching out, Richard @richard67. I need an expert opinion.

I have a step that needs to be inserted between 2 steps and the order may have been changed (for whatever reason).

The order of steps is recorded in the ordering column.

In the event the order of the steps had not been changed since install, it would go between steps with ids 5 and 6 (which have an ordering of 5 and 6 respectively). Would give the new step an order of 5 be enough?

avatar richard67
richard67 - comment - 19 Sep 2024

The order of steps is recorded in the ordering column.

In the event the order of the steps had not been changed since install, it would go between steps with ids 5 and 6 (which have an ordering of 5 and 6 respectively). Would give the new step an order of 5 be enough?

@obuisard I did not see the ordering column being used in your insert statements in the update SQL scripts. That’s why I was not aware of it.

when inserting something new between ordering 5 and 6, you can first update all steps with ordering >= 6 to set ordering = ordering + 1, so what had 6 before will have 7, what had 7 before will have 8 and so on, and ordering 6 would be free. Then insert the new step with ordering 6.

In general, when you do an „update sometable set somevalue=somevalue+1 where somecondition“, you increment somevalue by 1 where somecondition is true.

avatar obuisard
obuisard - comment - 19 Sep 2024

@obuisard I did not see the ordering column being used in your insert statements in the update SQL scripts. That’s why I was not aware of it.

when inserting something new between ordering 5 and 6, you can first update all steps with ordering >= 6 to set ordering = ordering + 1, so what had 6 before will have 7, what had 7 before will have 8 and so on, and ordering 6 would be free. Then insert the new step with ordering 6.

In general, when you do an „update sometable set somevalue=somevalue+1 where somecondition“, you increment somevalue by 1 where somecondition is true.

Thanks, Richard @richard67

avatar obuisard
obuisard - comment - 19 Sep 2024

Thank you, Richard @richard67 for your help

avatar obuisard obuisard - change - 19 Sep 2024
The description was changed
avatar obuisard obuisard - edited - 19 Sep 2024
avatar brianteeman
brianteeman - comment - 19 Sep 2024

technically this update query would not be correct if the user has changed the order of the steps but as thats very unlikely I can confirm the update sql works on mysql

avatar Kostelano
Kostelano - comment - 20 Sep 2024

No, something is not working.
As soon as you complete a tour (create a new one) and enable automatic start, the site will stop working. You can enable ALL components (then the start will stop working immediately) or you can select a specific component - then when you go to the component page, the site will stop working (404).

All newly created tours are placed in the Dashboard, although I have installed other components.

Screenshot_1

avatar obuisard
obuisard - comment - 20 Sep 2024

No, something is not working. As soon as you complete a tour (create a new one) and enable automatic start, the site will stop working. You can enable ALL components (then the start will stop working immediately) or you can select a specific component - then when you go to the component page, the site will stop working (404).

All newly created tours are placed in the Dashboard, although I have installed other components.

I have installed the PR as a fresh install, and as an update, created new tours, with auto starting enabled and did not seem to encounter any of those issues.
Was your install a fresh install? An update? Did you see any Database warning in the console?

The 404 error you are getting: what is the URL in your browser for that page? Could it be that the URL parameter you have set in the tour is incorrect (basically the URL is set to start in a page that does not exist)?

image

The tours you have created: are 123, 234, 455 the titles of the tours?

If you left 'Component selector' set to 'all', the tours will show in the Dashboard section (in the list of all the tours). Although maybe, as an improvement, we should put those special cases under no section... But it is unrelated to this PR.

image

If you create a tour for banners, you would select 'Banners' for the tour to show under the 'Banners' section.

If you have 3 tours set to auto-start and set to start anywhere (= when the component selector is set to 'all'), any refresh of any console page will start a new tour after you have either completed, cancelled or skipped the previous one.

avatar Kostelano Kostelano - test_item - 20 Sep 2024 - Tested successfully
avatar Kostelano
Kostelano - comment - 20 Sep 2024

I have tested this item ✅ successfully on 1bb95a1

I found a bug (an extra character in the URL that threw me off my feet), so I apologize for making you waste time looking for a non-existent bug, it happens ;).

Well, I tested PR with the old installation + service pack with PR and separately with the new installation. No errors were noticed, below I am posting a successful test. Thanks.


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

avatar exlemor exlemor - test_item - 20 Sep 2024 - Tested successfully
avatar exlemor
exlemor - comment - 20 Sep 2024

I have tested this item ✅ successfully on 1bb95a1

I have tested this item ✅ successfully.


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

avatar richard67 richard67 - change - 20 Sep 2024
Status Pending Ready to Commit
avatar richard67
richard67 - comment - 20 Sep 2024

RTC


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

avatar Quy Quy - change - 20 Sep 2024
Labels Added: RTC
avatar richard67
richard67 - comment - 21 Sep 2024

@alikon The PR has 2 good tests so I've set it RTC, but it would be good to get an additional quick test with PostgreSQL, too, just to see if installation with and updating to the patched package of this PR works. I would be happy if you could find the time. Thanks in advance.

avatar alikon
alikon - comment - 22 Sep 2024

update from 5.1.4 to Prebuilt package of this pr doesn't work

image

INSERT INTO "#__action_logs_extensions" ("extension") VALUES ("com_guidedtours"); coming from #43814

avatar richard67
richard67 - comment - 22 Sep 2024

@obuisard On PostgreSQL, double quotes are for names (table names, column names) but not for strings. Strings are quoted with single quotes.

avatar obuisard
obuisard - comment - 22 Sep 2024

@obuisard On PostgreSQL, double quotes are for names (table names, column names) but not for strings. Strings are quoted with single quotes.

Thanks Richard @richard67 , yes, I missed it. Thanks Nicola @alikon for the correction.

avatar richard67 richard67 - alter_testresult - 22 Sep 2024 - Kostelano: Tested successfully
avatar richard67 richard67 - alter_testresult - 22 Sep 2024 - exlemor: Tested successfully
avatar Hackwar Hackwar - change - 23 Sep 2024
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2024-09-23 07:48:21
Closed_By Hackwar
avatar Hackwar Hackwar - close - 23 Sep 2024
avatar Hackwar Hackwar - merge - 23 Sep 2024
avatar Hackwar
Hackwar - comment - 23 Sep 2024

Thank you for your contribution @obuisard!

Add a Comment

Login with GitHub to post a comment