Skip to content

feat: 2.0 Decouple the CRUD operation#4

Merged
coolbung merged 5 commits intotechjoomla:release-2.0.0from
thite-amol:master
Apr 19, 2018
Merged

feat: 2.0 Decouple the CRUD operation#4
coolbung merged 5 commits intotechjoomla:release-2.0.0from
thite-amol:master

Conversation

@thite-amol
Copy link
Contributor

Summary of Changes

These changes decouple the CRUD operation from users.php.

Also, following areas are improved

  1. Added ACL support for all operations
  2. Added support to identified user by using the username, email or userid.
  3. Added support to assign usergroups to the user.

Copy link
Member

@manojLondhe manojLondhe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check all comments @thite-amol


//send registration mail
/**
* send registration mail
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Send

Copy link
Contributor Author

@thite-amol thite-amol Apr 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The users.php file will revamp and deprecated functions will be removed from this file. I have just added a deprecated tag in it. Also the file is not PHPCS this will be done when we revamp this file.


//create field array as per easysocial
/**
* create field array as per easysocial
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create

/**
* Function delete for user record.
*
*@deprecated 2.0 use UsersApiResourceUser delete instead
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

* @deprecated

@@ -30,13 +30,15 @@
* @package Joomla.Administrator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change everywhere

@package     Com.Api
@subpackage  users

/**
* Function retriveUser for get user details depending upon the identifier.
*
* @param string $xidentifier Flag to differnciate the column value.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

differentiate

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

*
* @param Object $user The user object.
* @param Array $formData Array of user data to be added or updated.
* @param Boolean $flag Flag to differnciate the update of create action.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

differentiate

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

*
* @since 2.0
*/
private function storeUser($user, $formData, $flag = 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use isNew instead of flag? and change condition accordingly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

private function getUserId($email)
{
// Initialise some variables
$db = JFactory::getDbo();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove extra space after $db

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}

/**
* Function to returns userid if a user exists depending on email
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

*/
public function post()
{
$app = JFactory::getApplication();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you equi-align these lines, its hard to read?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@thite-amol thite-amol changed the base branch from master to release-2.0.0 April 12, 2018 13:35
$app = JFactory::getApplication();
$userIdentifier = $app->input->get('id', 0, 'String');
$formData = array();
$formData['username'] = $app->input->get('username', 0, 'String');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't this all be done with a single getArray() ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

$user = new JUser;

// Create new user.
$response = $this->storeUser($user, $formData, 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the ACL check for saving new user ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we have ACL check for registration in Joomla

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not ACL.. I meant the check to enable / disable new user creation.

$response = new stdClass;

$xidentifier = $app->input->server->get('HTTP_IDENTIFIER');
$fidentifier = $app->input->server->get('HTTP_FOURCECREATE');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong spelling. Should be HTTP_FORCECREATE

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}
else
{
if ($fidentifier)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this a different code path ? This is similar to creating a new user.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not clear on this. Will check this with you

@coolbung coolbung merged commit b3e2ebc into techjoomla:release-2.0.0 Apr 19, 2018
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.

3 participants