? Pending

User tests: Successful: Unsuccessful:

avatar joomdonation
joomdonation
17 Apr 2017

Pull Request for Issue # .

Summary of Changes

This is a propose fix for default $prefix parameter use in MvcFactory in Joomla 4. I know this is an ugly fix but could not find a better solution because we have to keep B/C with legacy MVC.

This needs code review and decision from Joomla 4 team.

Testing Instructions

  1. Install Joomla 4 staging
  2. Apply patch
  3. Play with com_content and make sure it works (backend)
  4. Play with a none namespace component (banners or contacts) and make sure it works
avatar joomdonation joomdonation - open - 17 Apr 2017
avatar joomdonation joomdonation - change - 17 Apr 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 17 Apr 2017
Category Libraries
avatar joomdonation joomdonation - change - 17 Apr 2017
Labels Added: ?
avatar joomdonation
joomdonation - comment - 17 Apr 2017

I also changed default prefix for Table object, too. I have to set it to Table for legacy factory to keep B/C. I don't understand why the legacy method MVC default it to Table, it makes every model has to override getTable method like this https://github.com/joomla/joomla-cms/blob/4.0-dev/administrator/components/com_banners/models/banner.php#L282

If it is allowed to break B/C, we should change that default prefix to ucfirst(substr($option, 4)).'Table'. If we can do that, even component which use none namespace MVC can remove the method getTable from it's Model classes.

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 17 Apr 2017

with or -out PR get after "Save"-Article:
bildschirmfoto 2017-04-17 um 09 06 27

avatar joomdonation
joomdonation - comment - 17 Apr 2017

Could you please try to download latest version here https://github.com/joomla/joomla-cms/tree/4.0-dev , then install and test it again? I could not re-procedure this issue

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 17 Apr 2017

Installed new using https://github.com/joomla/joomla-cms/tree/4.0-dev and Sample Content – same Error is showing after "Save" an Sample-Article.

avatar joomdonation
joomdonation - comment - 17 Apr 2017

@franz-wohlkoenig You are correct. This is a typo (unrelated to this PR) and should be fixed with #15346

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 17 Apr 2017

with or -out PR get after created an Tag in Article-View and "Save"-Article:
bildschirmfoto 2017-04-17 um 11 05 57

avatar joomdonation
joomdonation - comment - 17 Apr 2017

@franz-wohlkoenig Could you please open a new issue for that, please? Saving tag error should be a separate PR

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 17 Apr 2017

done #15347

avatar wilsonge
wilsonge - comment - 17 Apr 2017

Now that is fixed doesit work?

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 17 Apr 2017

@wilsonge works, no Error on saving tagged Article.

avatar joomdonation
joomdonation - comment - 17 Apr 2017

@franz-wohlkoenig Could you perform a full test as described in the test instructions? Need to test add/edit/delete/publish/unpublish in both com_content and com_contact

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 17 Apr 2017

i'm doing a test but struggle on other issues like #15349.

"add/edit/delete/publish/unpublish" is easier to Test than "Play with com_content and make sure it works".

avatar joomdonation
joomdonation - comment - 17 Apr 2017

I think you can ignore that select user step. Edit an article, save it, if the change is saved, then it works OK

Then try to delete an article, publish an article, unpublish.

Do the same for com_contact. If it works well, then the test is success

Sorry for an unclear test instructions

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 17 Apr 2017

@joomdonation Article: Changed Title and a Word in Fulltext > Title is saved, Fulltext not (using Default-Editor > TinyMCE).

avatar joomdonation
joomdonation - comment - 17 Apr 2017

Look like related to this issue #14760 and it is un-related to this PR. Could you try delete/publish/unpublish?

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 17 Apr 2017

Tests using Sample Sites.

Unpublished

  • Open Article:
    Change Status from Published to Unpublished is in Dropdown-Field saved, but:
    bildschirmfoto 2017-04-17 um 13 13 42
    Reopen Article, Status is Published.
  • List of Articles:
    Click on green Symbol/red Symbol work.

Delete

  • Open Article:
    Change Status from Published to Trashed is in Dropdown-Field saved, but:
    bildschirmfoto 2017-04-17 um 13 13 42
    Reopen Article, Status is Published.
  • List of Articles:
    Click on "Trash"-Symbol works, also show trashed Items by "Search Tools" and change Status back to published.
avatar joomdonation
joomdonation - comment - 17 Apr 2017

Thanks @franz-wohlkoenig . I guess this issue happens with and without this PR, so if it is possible, please open a separate issue for it

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 17 Apr 2017

Issue opened: #15350

avatar joomdonation
joomdonation - comment - 17 Apr 2017

Thanks. If you can perform the same test with com_contact, that would be great

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 17 Apr 2017

Com_contact works on delete/publish/unpublish/archived as expected.

avatar joomdonation
joomdonation - comment - 17 Apr 2017

OK. Thanks. You can mark the test as success. I will need to ask for code review / decision from Joomla 4 team with this change

avatar franz-wohlkoenig franz-wohlkoenig - test_item - 17 Apr 2017 - Tested successfully
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 17 Apr 2017

I have tested this item successfully on c8c9f5a


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

avatar franz-wohlkoenig franz-wohlkoenig - change - 17 Apr 2017
Easy No Yes
avatar joomdonation
joomdonation - comment - 17 Apr 2017

Thanks again @franz-wohlkoenig

avatar joomdonation
joomdonation - comment - 22 Apr 2017

@wilsonge Could you have a look at this PR?

avatar wilsonge wilsonge - change - 22 Apr 2017
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-04-22 15:42:13
Closed_By wilsonge
avatar wilsonge wilsonge - close - 22 Apr 2017
avatar wilsonge wilsonge - merge - 22 Apr 2017
avatar joomdonation
joomdonation - comment - 22 Apr 2017

Thanks ;)

avatar wilsonge
wilsonge - comment - 22 Apr 2017

Thankyou :)

Add a Comment

Login with GitHub to post a comment