Skip to content

[5.3] Clean up com_templates StyleModel#45077

Merged
laoneo merged 4 commits intojoomla:5.3-devfrom
joomdonation:com_templates_style_model
Mar 9, 2025
Merged

[5.3] Clean up com_templates StyleModel#45077
laoneo merged 4 commits intojoomla:5.3-devfrom
joomdonation:com_templates_style_model

Conversation

@joomdonation
Copy link
Contributor

Pull Request for Issue # .

Summary of Changes

This PR makes several clean up to com_templates StyleModel:

  • Remove populateState method, it does not do anything different than the code in parent class, so the override is useless
  • Improve getItem method. Instead of copying code from parent class, we just call the method from parent class to get item data, then add additional logic to it.
  • Improve preprocessForm code, prevent potential warnings when saving item failure (in that case, $data is an array, so we could not treat it as an object like how we do in the code at the moment)

Testing Instructions

  1. Use Joomla 5.3
  2. Apply patch
  3. Access to System -> Template Styles, edit a template style and make sure it is still working as expected

Actual result BEFORE applying this Pull Request

Works, but there is potential warning when saving item false (not easy to show it to you)

Expected result AFTER applying this Pull Request

Works, with less, cleaner code and no potential warnings.

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

@alikon
Copy link
Contributor

alikon commented Mar 7, 2025

I have tested this item ✅ successfully on 24d923d


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

@joomdonation
Copy link
Contributor Author

Thanks @alikon

@muhme
Copy link
Contributor

muhme commented Mar 7, 2025

I have tested on fc4338fbe3 with only the System Tests. Before applying this PR there are PHP Warnings:

  • api/com_templates/Site.cy.js
PHP Warning:  Attempt to read property "parent" on array in /var/www/html/administrator/components/com_templates/src/Model/StyleModel.php on line 399
PHP Warning:  Attempt to read property "parent" on array in /var/www/html/administrator/components/com_templates/src/Model/StyleModel.php on line 400
PHP Warning:  Attempt to read property "parent" on array in /var/www/html/administrator/components/com_templates/src/Model/StyleModel.php on line 399
PHP Warning:  Attempt to read property "parent" on array in /var/www/html/administrator/components/com_templates/src/Model/StyleModel.php on line 400
  • and api/com_templates/Administrator.cy.js
PHP Warning:  Attempt to read property "parent" on array in /var/www/html/administrator/components/com_templates/src/Model/StyleModel.php on line 399
PHP Warning:  Attempt to read property "parent" on array in /var/www/html/administrator/components/com_templates/src/Model/StyleModel.php on line 400

✅ After applying this PR the two tests are passing successfully and without PHP warnings.

@joomdonation
Copy link
Contributor Author

joomdonation commented Mar 8, 2025

Thanks @muhme. This PR also modifies code in different methods inside the StyleModel class, beside System tests, we also need to do real test to make sure template style can still be updated without error.

@ghost
Copy link

ghost commented Mar 8, 2025

@joomdonation Is it enough to modify a template style and save? Or mean the instructions another way saying "potential warning when saving item false (not easy to show it) (…) Works (…) no potential warnings".

@joomdonation
Copy link
Contributor Author

Thanks @fgsw. Just edit template style, save it and make sure that it is still working like before (mean nothing is broken with updating a template style)

@ghost
Copy link

ghost commented Mar 8, 2025

I have tested this item ✅ successfully on 043c7ad

Changed all colour fields settings, added an image to each image field.

@joomdonation
Copy link
Contributor Author

RTC. Thanks for testing!


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 9, 2025
@laoneo laoneo merged commit 0859c0f into joomla:5.3-dev Mar 9, 2025
3 of 4 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 9, 2025
@laoneo
Copy link
Member

laoneo commented Mar 9, 2025

Thanks!

@laoneo laoneo added this to the Joomla! 5.3.0 milestone Mar 9, 2025
@joomdonation
Copy link
Contributor Author

Thanks everyone !

@joomdonation joomdonation deleted the com_templates_style_model branch March 9, 2025 08:05
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.

7 participants