? Pending

User tests: Successful: Unsuccessful:

avatar bembelimen
bembelimen
13 Jul 2017

Pull Request for Issue #17001 .

Summary of Changes

This patch makes sure, that forms are not created twice, because of the data loading parameter. See: #17001 and #17043 (comment)

Testing Instructions

#17001 (comment) This patch should fix this behavior

Expected result

Plugins work correctly

Trigger: @infograf768 @Spudley

avatar bembelimen bembelimen - open - 13 Jul 2017
avatar bembelimen bembelimen - change - 13 Jul 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 13 Jul 2017
Category Libraries
avatar infograf768 infograf768 - change - 14 Jul 2017
Title
Fix #17001
REGRESSION: Profile form bug when registering Fix #17001
avatar infograf768 infograf768 - edited - 14 Jul 2017
avatar infograf768 infograf768 - test_item - 14 Jul 2017 - Tested successfully
avatar infograf768
infograf768 - comment - 14 Jul 2017

I have tested this item successfully on 2c3d04a


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

avatar infograf768 infograf768 - change - 14 Jul 2017
Priority Medium Urgent
avatar AlexRed AlexRed - test_item - 14 Jul 2017 - Tested successfully
avatar AlexRed
AlexRed - comment - 14 Jul 2017

I have tested this item successfully on 2c3d04a

Patch ok for me


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

avatar infograf768 infograf768 - change - 14 Jul 2017
Milestone Added:
Status Pending Ready to Commit
avatar infograf768
infograf768 - comment - 14 Jul 2017

RTC.

Please @rdeutz merge before RC


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

avatar rdeutz
rdeutz - comment - 17 Jul 2017

@nikosdion could you have a look at this, thanks!

avatar nikosdion
nikosdion - comment - 17 Jul 2017

This looks pretty safe and fixes a long-standing issue. Forms should be instantiated once per model.

As a further step to avoid similar regressions in the future I would recommend that the leadership documents the opaque $options array and links to the documentation from the docblock of the method.

If I were to be pedantic I'd say that ideally we should be passing immutable objects instantiated against final classes (therefore: with predefined properties and with data that can't change) to make the internal API self-documenting and consistent. But since this course of action would be breaking b/c, in a package marked legacy nonetheless, I think we can safely assume this isn't happening in the near future. So at least documenting opaque parameters would be a good step towards avoiding similar regressions in the future.

avatar rdeutz rdeutz - change - 17 Jul 2017
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-07-17 11:40:27
Closed_By rdeutz
Labels Added: ?
avatar rdeutz rdeutz - close - 17 Jul 2017
avatar rdeutz rdeutz - merge - 17 Jul 2017
avatar rdeutz
rdeutz - comment - 17 Jul 2017

Thanks for all worked and commented on this :-)

Add a Comment

Login with GitHub to post a comment