? Success

User tests: Successful: Unsuccessful:

avatar csthomas
csthomas
7 Mar 2016

Explanation per files:

  • components/com_users/models/profile.php

    Do not call more than once $this->loadFormData() which call getItem() and preprecessData() each time. getItem() should not trigger onContentPrepareData. The events should be triggered in preprocessData() only.

  • administrator/components/com_users/models/user.php
    getItem() is call twice in legacy view:

    view::get('Item');
    view::get('form');

    so cache it and do not run twice the same code (2 sql queries twice = 4 queries)

    Replace ->where($db->qn('id') . ' = ' . $db->q($user_id));
    to ->where($db->qn('id') . ' = ' . (int) $user_id); which will be the same as in Juser class.
    This is for mysql which can cache the query if it has the same text.

Summary of Changes

  • Do not trigger onContentPrepareData more than once.
  • Reduce sql queries.

Testing Instructions 1

  1. Download and Install the latest Joomla 3 with sample data.
  2. Install com_patchtester.zip
  3. Enable plugin "User - Profile"
  4. Add a few lines in /plugins/user/profile/profile.php.

At the top of method onContentPrepareData line ~ 61:

echo "<p><b>onContentPrepareData<b> was run with context $context from<br><code>";
array_walk(debug_backtrace(false),create_function('$a,$b','echo "{$a[\'function\']}()(".$a[\'file\'].":{$a[\'line\']}); ";'));
echo "</code></p>";

and at the top if method onContentPrepareForm line ~ 233:

echo "<p><b>onContentPrepareForm<b> was run from<br><code>";
array_walk(debug_backtrace(false),create_function('$a,$b','echo "{$a[\'function\']}()(".$a[\'file\'].":{$a[\'line\']}); ";'));
echo "</code></p>";

This way we can see how many times that methods is run.

This PR reduce calls method onContentPrepareData.

  1. Login and go to profile view /index.php?option=com_users&view=profile
    You should see:
    user1

  2. Go to edit profile:
    user2

  3. Edit file /libraries/legacy/controller/legacy.php and comment line 947 as:

            //$app->redirect($this->redirect);
  1. Save your profile and you see:
    user3

  2. Uncomment line from point 7)

  3. Back to backend to /administrator/index.php?option=com_patchtester
    Search for 9325 or "User Profile - remove redundant event trigger onContentPrepareData"
    Then apply that patch.

  4. Go to profile view /index.php?option=com_users&view=profile
    You should see now:
    user4

  5. Go to edit profile and you should see:
    user5

  6. Comment line from point 7)

  7. Save your profile
    user6

avatar csthomas csthomas - open - 7 Mar 2016
avatar csthomas csthomas - change - 7 Mar 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 7 Mar 2016
Labels Added: ?
avatar brianteeman brianteeman - change - 26 Mar 2016
Category Code style
avatar brianteeman brianteeman - change - 11 May 2016
Category Code style Code style Components Libraries
avatar joomla-cms-bot joomla-cms-bot - change - 28 Jul 2016
Category Code style Components Libraries Administration Components Front End Libraries Code style
avatar csthomas csthomas - change - 28 Jul 2016
Title
User Profile - remove redundant event trigger onContentPrepareData, read data from session after validation fails
User Profile - remove redundant event trigger onContentPrepareData
avatar csthomas csthomas - change - 28 Jul 2016
Title
User Profile - remove redundant event trigger onContentPrepareData, read data from session after validation fails
User Profile - remove redundant event trigger onContentPrepareData
avatar csthomas csthomas - edited - 28 Jul 2016
avatar csthomas csthomas - change - 28 Jul 2016
The description was changed
avatar csthomas csthomas - edited - 28 Jul 2016
avatar wmchris wmchris - test_item - 2 Aug 2016 - Tested unsuccessfully
avatar wmchris
wmchris - comment - 2 Aug 2016

I have tested this item ? unsuccessfully on

Applied the changes, dont get a difference before and after patch in the messages

'''
on open:
I was run in 63
I was run in 230

on changing two or more fields:
I was run in 230
User successfully saved.
I was run in 63
'''


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

avatar schmidtpaddy schmidtpaddy - test_item - 2 Aug 2016 - Tested unsuccessfully
avatar schmidtpaddy
schmidtpaddy - comment - 2 Aug 2016

I have tested this item ? unsuccessfully on

test didn't changed @icampus


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

avatar roland-d
roland-d - comment - 2 Aug 2016

@csthomas I wonder if these failed tests are really failed or it is unclear what to test. Reading the test instructions I am not sure what to test. Your summary of changes state what is supposed to changed. Perhaps we can clarify the test instructions with how often the echo should be shown and where to check in the debug information to see the query gone?


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

avatar csthomas
csthomas - comment - 2 Aug 2016

Thanks @roland-d. This PR was separated to few PRs. Two of it was merged. This stay because it is more complicated to test.

I will prepare a better test instruction today evening.

avatar csthomas csthomas - edited - 2 Aug 2016
avatar csthomas
csthomas - comment - 2 Aug 2016

I have updated test instruction. Please test again.

avatar roland-d
roland-d - comment - 3 Aug 2016

@schmidtpaddy @wmchris Can you please again using the new test instructions? Thank you.

avatar csthomas
csthomas - comment - 7 Sep 2016

@rdeutz Can it be merged with 3.6.x after tests success?

avatar rdeutz
rdeutz - comment - 7 Sep 2016

@csthomas could you please stop mention me on anything

avatar csthomas csthomas - change - 19 Oct 2016
The description was changed
avatar csthomas csthomas - edited - 19 Oct 2016
avatar joomla-cms-bot joomla-cms-bot - change - 19 Oct 2016
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 19 Oct 2016
Category Code style Components Libraries Administration Front End Administration Components Front End Code style
avatar brianteeman brianteeman - change - 29 Oct 2016
Labels Removed: ?
avatar microtribe
microtribe - comment - 13 Nov 2016

I have tested this item successfully on 5769c3d

Ran updated test as described and got the same results at each stage - made edits to /plugins/user/profile/profile.php - completed each of the other steps described, applied the patch, same results.


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

avatar microtribe microtribe - test_item - 13 Nov 2016 - Tested successfully
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 6 Jan 2017

I have tested this item successfully on 5769c3d

Got same Result as written in Test Instructions.


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

avatar franz-wohlkoenig franz-wohlkoenig - test_item - 6 Jan 2017 - Tested successfully
avatar jeckodevelopment
jeckodevelopment - comment - 6 Jan 2017

@Bakual / @zero-24 can we go RTC for this?

avatar Bakual
Bakual - comment - 6 Jan 2017

@jeckodevelopment I have no clue what this does here. As long as the models behave the same and trigger the plugin events, I'd say it should be fine, but I only glanced over it.

avatar csthomas
csthomas - comment - 10 Jan 2017

I have added a few more explanation to code to be more easy to understand the changes.

avatar csthomas csthomas - change - 30 Jan 2017
The description was changed
avatar csthomas csthomas - edited - 30 Jan 2017
avatar joomla-cms-bot joomla-cms-bot - change - 30 Jan 2017
Category Code style Components Administration Front End Administration com_users Front End Code style Components
avatar csthomas
csthomas - comment - 30 Jan 2017

What the status of that PR, RTC?

avatar jeckodevelopment jeckodevelopment - change - 30 Jan 2017
Status Pending Ready to Commit
avatar jeckodevelopment
jeckodevelopment - comment - 30 Jan 2017

RTC


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

avatar rdeutz rdeutz - change - 11 Feb 2017
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-02-11 11:38:13
Closed_By rdeutz
Labels Added: ?
avatar rdeutz rdeutz - close - 11 Feb 2017
avatar rdeutz rdeutz - merge - 11 Feb 2017

Add a Comment

Login with GitHub to post a comment