Skip to content

Conversation

@joomdonation
Copy link
Contributor

Pull Request for Issue # .

Summary of Changes

This PR makes some minor clean up to Step model from guided tours component :

  1. Simplify code of canEditState method. We do not have to set tour_id for the record before checking the permission. The only case we have to do that is when adding a new step, but this is already being taken care by getItem method
  2. Simplify save method. We do not have get tour_id from session and set it for step. The value for that field is already passed from hidden variable on step form (and we already validate and make sure it's available when a step is being added/editted)
  3. Remove some one time used variable
  4. Rename variable $tourID to $tourId to follow camel case naming convention

Testing Instructions

  1. Use Joomla 4.3 nightly build or a Joomla 4.3 installation which is having Tours Guided available
  2. Apply patch. Try to add/edit step for a tour, make sure nothing is broken

Actual result BEFORE applying this Pull Request

Works

Expected result AFTER applying this Pull Request

Works with cleaner code

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

@obuisard
Copy link
Contributor

obuisard commented Mar 9, 2023

I have not tested this item.


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

@obuisard
Copy link
Contributor

obuisard commented Mar 9, 2023

I have tested this item ✅ successfully on efe058d


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

1 similar comment
@khu5h1
Copy link
Contributor

khu5h1 commented Mar 9, 2023

I have tested this item ✅ successfully on efe058d


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

@Quy
Copy link
Contributor

Quy commented Mar 9, 2023

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 9, 2023
@obuisard obuisard added this to the Joomla! 4.3.0 milestone Mar 9, 2023
@obuisard obuisard merged commit 1c0690c into joomla:4.3-dev Mar 9, 2023
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 9, 2023
@obuisard
Copy link
Contributor

obuisard commented Mar 9, 2023

Thank you Tuan @joomdonation, appreciated!

@joomdonation joomdonation deleted the guided_tours_cleanup_step_model branch March 10, 2023 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants