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

[4.0] Use MVCFactory to create model #34092

Merged
merged 2 commits into from
May 30, 2021

Conversation

joomdonation
Copy link
Contributor

Pull Request for Issue # .

Summary of Changes

In Joomla 4, the right way to create a model object is use MVCFactory after bootComponent. For example https://github.com/joomla/joomla-cms/blob/4.0-dev/administrator/components/com_categories/src/Helper/CategoriesHelper.php#L99

However, there are still some places in our code use old method (create model class directly using new ModelClassName syntax). This PR improves code to make it consistent in our whole codebase.

Testing Instructions

The modified code is random in our CMS, so I think the best way is someone with code review skill to review the code and make sure it is all correct. Or maybe @wilsonge will need to review and merge base on code review.

@richard67
Copy link
Member

@joomdonation There are PHP code style errors: https://ci.joomla.org/joomla/joomla-cms/44216/1/6

@wilsonge
Copy link
Contributor

I’m happy with most places on review.

Only thing I wouldn’t mind tested is the script.php part doing a J3 to j4 upgrade - just because I don’t know exactly where that runs in the upgrade cycle and whether we’ll be exclusively using the j4 files everywhere (I think it will be fine - but better safe than sorry)

@joomdonation
Copy link
Contributor Author

joomdonation commented May 23, 2021

@wilsonge I installed Joomla 3.10.0 nightly, tested the update process and it is working well, so I think it is fine.

For technical information, the code which is changed here will be executed when finaliseUpgrade method of UpdateModel is called. It is called when:

If it is needed, I will remove the code which I changed in com_admin/script.php to be safe. Let me know if you want me to do that.

@wilsonge wilsonge merged commit 9f8f1c6 into joomla:4.0-dev May 30, 2021
@wilsonge
Copy link
Contributor

I’m happy with that explanation. Thanks!

@wilsonge wilsonge added this to the Joomla 4.0 milestone May 30, 2021
@joomdonation joomdonation deleted the create_models branch May 30, 2021 03:20
bembelimen pushed a commit that referenced this pull request Nov 23, 2021
* [4.0] Notification icons (#34226)

* remove method allow parent to run (#34118)

Co-authored-by: Richard Fath <[email protected]>

* [4.0] Changing title to tooltip for template preview (#33292)

* [4.0] Changing title to tooltip fpr template preview

* Update location path in description (#34238)

* Fix patterns field check when field empty and not required (#34124)

* [4.0] Finish transition from CSS classes "label-" to "alert-" for the pre-update check (#34227)

* [4.0] mod_popular with disabled hits (#34257)

* [4.0] mod_popular with disabled hits

When hits are disabled for articles it makes no sense to display the Most Read Articles module in the site or admin as the contents will never be updated.

This PR changes the output of the module so that instead of a list of non-updating articles a message is displayed.

* [4.0] Use MVCFactory to create model (#34092)

* Use MVCFactory to create models

* CS

* Fix row selecting, when module is disabled (#34273)

* Media web service implementation.

* Media web service implementation.

* Fix hard coded adapter name default.

* Fix missing url and tempUrl attributes in single item response.

* Remove useless comments.

* Replace 'PATH' with 'STRING' for cleaning of input parameters.

* Adds required parameter checks and removes global exception handling.

* Adds media web service specific language strings.

* Adds exception handling.

* Adds missing exception handler and fixes wrong path return value after move/rename.

* Use exception message as error title.

* Adds proper FileNotFound exception handling for GET requests.

* Fixes handling of status code.

* Fixes some types, formatting and similar stuff.

* Fixes yet another sloppy typo.

* restore file

* Fix some docs

* Adapter endpoint

* Add plugin to install file

* Add tests

* Update api/components/com_media/src/Helper/MediaHelper.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/Helper/MediaHelper.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/Model/MediaModel.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/Model/MediumModel.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/View/Media/JsonapiView.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update libraries/src/Error/JsonApi/SaveExceptionHandler.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/Controller/MediaController.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/Controller/MediaController.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/Model/AdaptersModel.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/Model/MediaModel.php

Co-authored-by: Phil E. Taylor <[email protected]>

* docs

* Update api/components/com_media/src/View/Media/JsonapiView.php

Co-authored-by: Phil E. Taylor <[email protected]>

* int

* cs

* cs

* more cleanup

* tabs

* restore

* Test all endpoints

* Update api/components/com_media/src/Model/MediaModel.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Hardening

* Tests fix for drone

* Update Api.php

* Create directory in test with correct permissions

* Add more tests and remove static helper

* Update tests/Codeception/api/com_media/MediaCest.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update tests/Codeception/_support/Helper/Api.php

Co-authored-by: Phil E. Taylor <[email protected]>

* cs

Co-authored-by: Brian Teeman <[email protected]>
Co-authored-by: Phil E. Taylor <[email protected]>
Co-authored-by: Richard Fath <[email protected]>
Co-authored-by: infograf768 <[email protected]>
Co-authored-by: Quy <[email protected]>
Co-authored-by: Ruud <[email protected]>
Co-authored-by: Tuan Pham Ngoc <[email protected]>
Co-authored-by: Fedir Zinchuk <[email protected]>
Co-authored-by: Pieter-Jan de Vries <[email protected]>
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.

4 participants