[GitHub] incubator-trafficcontrol issue #370: Adding a "create user" to the api

[mitchell852]

Github user mitchell852 commented on the issue:

https://github.com/apache/incubator-trafficcontrol/pull/370
  
yes, there is a UT failure:

t/user.t  (Wstat: 256 Tests: 21 Failed: 
1)
  Failed test:  5
  Non-zero exit status: 1

but i'm pretty sure this was fixed in master so i'll pull this in and if 
it's still broken, i can fix it.

it  only changes the behavior of user-update in the sense that role is now 
checked which in my opinion is a very valid check. you should not be able to 
update a user and leave out a required field - role.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol issue #370: Adding a "create user" to the api

[nir-sopher]

Github user nir-sopher commented on the issue:

https://github.com/apache/incubator-trafficcontrol/pull/370
  
The change of the "role" broke the UT and was reverted.
It changes current behavior of user-update.
I prefer not to make such changes in this PR


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol issue #370: Adding a "create user" to the api

[nir-sopher]

Github user nir-sopher commented on the issue:

https://github.com/apache/incubator-trafficcontrol/pull/370
  
Rebase was done a PR review changes were made, except for a test for 
"username" in the beginning of the create function. The test is done as the 
value is in use by the function logic.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol issue #370: Adding a "create user" to the api

[mitchell852]

Github user mitchell852 commented on the issue:

https://github.com/apache/incubator-trafficcontrol/pull/370
  
@nir-sopher - can you fix this PR? "This branch cannot be rebased safely" - 
 i'd like to get it merged if possible


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol issue #370: Adding a "create user" to the api

[mitchell852]

Github user mitchell852 commented on the issue:

https://github.com/apache/incubator-trafficcontrol/pull/370
  
you might want to run your code thru the perl formatter. I see some 
formatting errors. Here's more info about that:


https://github.com/apache/incubator-trafficcontrol/blob/master/CONTRIBUTING.md#code-formatting


http://trafficcontrol.apache.org/docs/latest/development/traffic_ops.html#perl-formatting-conventions


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol issue #370: Adding a "create user" to the api

[nir-sopher]

Github user nir-sopher commented on the issue:

https://github.com/apache/incubator-trafficcontrol/pull/370
  
Indeed

On Apr 7, 2017 6:16 AM, "Jeremy Mitchell"  wrote:

> *@mitchell852* commented on this pull request.
> --
>
> In traffic_ops/app/lib/API/User.pm
> 

> :
>
> > +   my $name = $params->{username};
> + if ( !defined($name) ) {
> + return $self->alert("Username is required.");
> + }
> + 
> + my $existing = $self->db->resultset('TmUser')->search( { username => 
$name } )->single();
> + if ($existing) {
> + return $self->alert("A user with username \"$name\" already 
exists.");
> + }
> +
> +
> + if ( !defined($params->{fullName}) ) {
> + return $self->alert("full-name is required.");
> + }
> +
> + if ( !defined($params->{email}) ) {
>
> nevermind, i guess email uniqueness is checked in the is_valid() function
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> 
,
> or mute the thread
> 

> .
>



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---