Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get-JiraUser is too helpful #49

Open
brianbunke opened this issue Sep 27, 2016 · 15 comments
Open

Get-JiraUser is too helpful #49

brianbunke opened this issue Sep 27, 2016 · 15 comments

Comments

@brianbunke
Copy link
Contributor

brianbunke commented Sep 27, 2016

Overview

Currently, Get-JiraUser always uses the /search method. I'd like it if there was an option to just grab a user by username, without searching.

Steps to Reproduce

Assume two users:1

  1. John Smith, username john
  2. John Smith, username smith

Get-JiraUser -UserName 'john'

Results

Actual

Name    DisplayName   Active
----    -----------   ------
john    John Smith    True
smith   John Smith    True

Expected (well...hoped for)

Name    DisplayName   Active
----    -----------   ------
john    John Smith    True

Background

I actually ran into this when I was trying to create my first issue, as in #28. Because New-JiraIssue is verifying all inputs, it was using Get-JiraUser to transform my simple, unassuming username string into a gorgeous, intricate, yet worthless array of two objects.

$userGetUrl already acknowledges the desired URI, but isn't called later in the current Get-JiraUser code.

Suggestions

There's two ways to fix this, if desired. The optimal method, IMHO, is also a breaking change. (Get-JiraUser grabs a username, and a new Find-JiraUser performs a search if desired.)

That leaves the solution for real people in real life, which is probably a switch parameter like -NoSearch or -Exact or something that hits the "get" URI instead of the "find" URI.

I can submit a PR if you agree.

Brown-nosing

This module is the best, and I'm sorry you're not working with JIRA day-to-day anymore. Thanks, I love you, stay gold, etc.


1 - I know this is a really dumb issue, exacerbated by a lame fictional naming convention. And it may be making me question my own sanity a little in sitting here filing it. But here I am anyway, and here you are, reading this while wondering the same thing, so I might as well try to make it a little enjoyable.

@replicaJunction
Copy link
Collaborator

Hah, this is one of my favorite issues. You, sir, have a great attitude, and I appreciate you...partly because you included a full section for brown-nosing. :)

I can understand your reasoning, and I agree that there should be a method to return a specific, known user, rather than searching for one. PowerShell already has a precedent for using the Get verb to return lists of things, including some wildcard operations (Get-Process, Get-Service, Get-ADUser, etc.), so I think I'm in agreement that an -Exact switch for Get-JiraUser would be the best solution.

UNLESS (dramatic music) we took a hybrid approach:

  1. Get-JiraUser first tries to identify the user by exact username
  2. If no results come back from JIRA, retry using the search method (current behavior)

Optionally, it could even search for wildcard chars like * or % in the username, and if those were detected, it could skip straight to step 2 above.

That way, there's no need for a separate parameter, and it would operate on something of a "best effort" search. In your example above, this behavior would return only the first John...um, Smith...well, with the username John.

What do you think?

@brianbunke
Copy link
Contributor Author

Yeah man, that hybrid option makes sense to me. Let me know if you'd like any help!

@aaronsb
Copy link

aaronsb commented Oct 4, 2016

Two thoughts:

Would a $PSCmdlet.MyInvocation test be appropriate here? For example, calling from a command line vs a function could cause one behaviour or the other.

Would this be an example where extending the data from Get-JiraConfigServer might be appropriate to contain other items such as cmdlet preferences, or some sort of preference behavior sets?

@replicaJunction
Copy link
Collaborator

Hmm...not sure I agree on different behavior between CLI and function / script file use. That could hurt the ease of use of the module when writing longer scripts that use the function - suddenly, the thing I just ran interactively in a console doesn't work the same way in a script file. Could you give me a bit more details on what you mean?

I'm also toying with the idea of removing the config file completely (#45), so I'm not sure about adding more data to it at the moment.

@aaronsb
Copy link

aaronsb commented Oct 4, 2016

@replicaJunction Yeah, I think you are right. Consistent behavior is more important than convenience in this case, since getting a user is a pretty core item. MyInvocation simply returns the calling function. If it's getting called from the console it may be empty, whereas calling from something else would return the name of the calling function.

I have written a lot of posh code that uses configuration files/sets/parameter locations and it's always a pita to manage it. I saw you addressed that in another location, lemmie read what you're thinking.

I think the primary concern I have with an intelligent return on user objects is some sort of partial match or malformed match failure.

Maybe add another noteproperty to get-jirauser that indicates a literal match of partial match of the user account. Otherwise it's hard to tell the difference between "auser" and "aus" in returns without additional calling logic. A nice to have I suppose.

replicaJunction added a commit that referenced this issue Oct 18, 2016
Previously, this function would always run a user search, even if the exact user existed.

This behavior can be overridden by passing the -AlwaysSearch parameter to the function.

Addresses #49.
@replicaJunction
Copy link
Collaborator

Just pushed a possible fix to a new branch, referenced above. Feel free to check it out if you guys would like; otherwise, I'll merge it back to master in a few days if no one has any issues with it. :)

@aaronsb I'm not sure I know what you mean. Here's an example with the new changes I just pushed.

Assume these users exist:

Username      Name   
--------      ----
jsmith        John Smith
jsmithjones   Jane Smith-Jones
jsmit         Jim "Schmitty" Smit

Given that (obviously manufactured) example, here's how a couple of commands would work:

# Returns one result, Jim Smit, because an exact match was found
Get-JiraUser jsmit

# Returns all of the above objects, because there was no exact match
Get-JiraUser jsmi

# Returns one result, John Smith, because of an exact match. Note that this doesn't return Jane
Get-JiraUser jsmith

# Force a search to return both John and Jane
Get-JiraUser jsmith -AlwaysSearch

Does that answer your concerns, or am I misunderstanding?

@brianbunke
Copy link
Contributor Author

Working as expected here. Thanks!

Failing the exact match causes Invoke-JiraMethod to return a generic 404 warning. Not desirable IMHO, but doesn't affect anything.

@replicaJunction
Copy link
Collaborator

Drat...I was hoping -ErrorAction SilentlyContinue would be enough to handle that. I might need to add a "suppress error" param to Invoke-JiraMethod. Glad to hear it's working otherwise, though!

@lipkau
Copy link
Member

lipkau commented Nov 8, 2016

Hi there.

Just wanted to add my opinion to the mix:
I prefer when you get-item only the exact match (username) and are able to -Full to get all (less precise) matches.

@lipkau
Copy link
Member

lipkau commented Dec 1, 2016

To elaborate: it is best practice, to have a defined (and consistent) object type that is returned with a given parameter set

Example:

The following advanced function uses the OutputType attribute to indicate that 
    the function returns different types depending on the parameter set used in the
    function command.

       function Get-User
       {
           [CmdletBinding(DefaultParameterSetName="ID")]

           [OutputType("System.Int32", ParameterSetName="ID")]
           [OutputType([String], ParameterSetName="Name")]

           Param (      
               [parameter(Mandatory=$true, ParameterSetName="ID")]
               [Int[]]
               $UserID,

               [parameter(Mandatory=$true, ParameterSetName="Name")]
               [String[]]
               $UserName
           )     
                  
         <function body>
       }

Source : https://technet.microsoft.com/en-us/library/hh847785.aspx

@axxelG
Copy link
Contributor

axxelG commented Jan 11, 2017

I like to add my opinion here as well: I don't like the way the JIRA api is working. I would prefer a way to choose between searching the username and the display name. So two parameters (-UserName and -DisplayName) would do it best. Both of them could behave 'powershellish' when they are working with exact matches unless a wildcard is used.

A small caveat will be that you have to filter the results within powershell after you got the search results. And a big one is, that this will brake backward compatibility and could become inconsistent between different functions.

@lipkau
Copy link
Member

lipkau commented Jan 13, 2017

maybe something like:

function Get-JiraUser
{
           [CmdletBinding(DefaultParameterSetName="singleUserName")]

           [OutputType([Jira.User], ParameterSetName="singleUserName")]
           [OutputType([Jira.User], ParameterSetName="singleDisplayName")]
           [OutputType([Jira.User[]], ParameterSetName="allUserName")]
           [OutputType([Jira.User[]], ParameterSetName="allDisplayName")]

           Param (      
               [parameter(Mandatory=$true, ParameterSetName="allUserName")]
               [parameter(Mandatory=$true, ParameterSetName="singleUserName")]
               [string[]]
               $UserName,

               [parameter(Mandatory=$true, ParameterSetName="allDisplayName")]
               [parameter(Mandatory=$true, ParameterSetName="singleDisplayName")]
               [string[]]
               $DisplayName,

               [parameter(Mandatory=$true, ParameterSetName="allDisplayName")]
               [parameter(Mandatory=$true, ParameterSetName="allUserName")]
               [switch]
               $All
         )

<...>
}

@axxelG
Copy link
Contributor

axxelG commented Jan 26, 2017

I found some time to precise my idea a bit.

Most cmdlets I know work in the following way:
No parameter given: give back all items
Parameter with wildcard: use wildcard query
Parameter without wildcard: search for exact match.

It could be done like this:

function makeTestUser ($UserName, $DisplayName) {
# needed only for testing my aproach
    $obj = New-Object -TypeName PSObject -Property @{
        Name = $UserName
        DisplayName = $DisplayName
        Active = $true
    }
    $obj.PSObject.TypeNames.Insert(0, 'PSJira.User')
    Write-Output $obj
}

function queryJiraApi ($Name) {
# needed only for testing my aproach
    Write-Output (makeTestUser -UserName 'UserA' -DisplayName 'User A')
    Write-Output (makeTestUser -UserName 'UserAA' -DisplayName 'User A')
    Write-Output (makeTestUser -UserName 'UserB' -DisplayName 'User B')
    Write-Output (makeTestUser -UserName 'UserC' -DisplayName 'User C')
}



function Get-JiraUserTest
{
    [CmdletBinding(DefaultParameterSetName="NoParameters")]

    Param (      
        [parameter(Mandatory=$true, ParameterSetName="UserName")]
        [string]
        $UserName,

        [parameter(Mandatory=$true, ParameterSetName="DisplayName")]
        [string]
        $DisplayName
    )

    PROCESS {
        $FilterName = "$UserName$DisplayName" -replace '\*'
        if ($PSCmdlet.ParameterSetName -eq 'NoParameters') {
            Write-Output (queryJiraApi)
        } else {
            Write-Output (queryJiraApi -Name $FilterName | where {($_.Name -like $UserName) -or ($_.DisplayName -like $DisplayName)})
        }
    }

}

But it will break backward compatibility as somebody could have used -UserName to search for a Display name and this won't work anymore. Maybe you can add -Search parameter to query like before but that could cause some refactoring as well.

@lipkau
Copy link
Member

lipkau commented Jul 8, 2017

@replicaJunction
How is this going?

Personally, I prefer separating Get and Find. Reason: Get-service -name "g*" only applies the search on a single field. So if Get-jiraUser john returns all users that have john in their username , and ignoring all other fields, I am on board.
But in order to search by last name, i'd like find-jirauser better

@lipkau
Copy link
Member

lipkau commented Apr 24, 2018

Proposal

# get self
Get-JiraUser

# get user by username "john"
# will return "john"
# accepts pipeline
Get-JiraUser -Name "john"

# get users with name "john"
# will return "john" and "smith"
Get-JiraUser -Filter "john"

Note

As this would change the behavior of how users as search for with the current parameter, this is a breaking change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants