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

Add 'vote' and 'unvote' #26

Merged
merged 3 commits into from
Jan 24, 2016
Merged

Add 'vote' and 'unvote' #26

merged 3 commits into from
Jan 24, 2016

Conversation

mikepea
Copy link
Collaborator

@mikepea mikepea commented Jan 13, 2016

This adds support for voting on issues via CmdVote() and CmdUnvote()

Voting on issues is always done as the logged in user, it appears you
can't case a vote for another user:

https://docs.atlassian.com/jira/REST/latest/#api/2/issue-addVote

This required adding a cli.delete() handler, naturally with no content
(as per RFC2616)

This is ripe for DRY-ing out, but I will leave that for a future PR.

Worth noting is that you cannot vote for your own issues, this results in:

2016-01-13T21:35:41.315Z ERROR [cli.go:184] response status: 404 Not Found
2016-01-13T21:35:41.315Z ERROR [commands.go:439] Unexpected Response From POST:
{snip}
{"errorMessages":["You cannot vote for an issue you have reported."],"errors":{}}

This adds support for voting on issues via CmdVote() and CmdUnvote()

Voting on issues is always done as the logged in user, it appears you
can't case a vote for another user:

https://docs.atlassian.com/jira/REST/latest/#api/2/issue-addVote

This required adding a cli.delete() handler, naturally with no content
(as per RFC2616)

This is ripe for DRY-ing out, but I will leave that for a future PR.

Worth noting is that you cannot vote for your own issues, this results in:

    2016-01-13T21:35:41.315Z ERROR [cli.go:184] response status: 404 Not Found
    2016-01-13T21:35:41.315Z ERROR [commands.go:439] Unexpected Response From POST:
    {snip}
    {"errorMessages":["You cannot vote for an issue you have reported."],"errors":{}}
@coryb
Copy link
Contributor

coryb commented Jan 14, 2016

Looks good. In the interest of minimizing commands, perhaps we could minimize the usage to:

jira vote ISSUE [--down]

where vote --down == unvote and would call the DELETE. I think that also fixes the DRY if we just pass a second arg to CmdVote(issue string, up bool) and the call would be something like CmdVote(args[0], !opts["down"].(bool))

Also I think you missed adding the command to the big usage text block.

I can fix these issues post merge if you like.
-Cory

@mikepea
Copy link
Collaborator Author

mikepea commented Jan 14, 2016

Sounds good - actually works better for me this way: the func defines how
to treat that API entity rather than how to perform a specific action.

Similarly, looking at CmdWatch, there's no code to unwatch - which I need.

Would you be up for me changing the spec of that to be:

func (c *Cli) CmdWatch(issue string, watcher string, up bool) error

... and push the population of 'watcher' up to main.go?

This works better for my needs for https://github.com/mikepea/go-jira-ui --
having to manipulate c.opts to call a Cmd*() is a pain (as I need to deal
with multiple actions in the same binary)

On Thu, Jan 14, 2016 at 6:39 AM, coryb [email protected] wrote:

Looks good. In the interest of minimizing commands, perhaps we could
minimize the usage to:

jira vote ISSUE [--down]

where vote --down == unvote and would call the DELETE. I think that also
fixes the DRY if we just pass a second arg to CmdVote(issue string, up
bool) and the call would be something like CmdVote(args[0],
!opts["down"].(bool))

Also I think you missed adding the command to the big usage text block.

I can fix these issues post merge if you like.
-Cory


Reply to this email directly or view it on GitHub
#26 (comment)
.

@coryb
Copy link
Contributor

coryb commented Jan 14, 2016

yeah, sounds good. I guess for CmdWatch we should change the last param to "add" (instead of "up"), so:

func (c *Cli) CmdWatch(issue string, watcher string, add bool) error

with usage:

jira watch ISSUE [--watcher user] [--remove]

Then we call with:
CmdWatch(args[0], opts["user"].(string), !opts["remove"].(bool))

-Cory

@coryb
Copy link
Contributor

coryb commented Jan 14, 2016

Well, i guess the lass call should be something like:

watcher := c.GetOptString("watcher", opts["user"].(string))
CmdWatch(args[0], watcher, opts["remove"].(bool))

or somethine, where GetOptsString looks for options and returns the second (default) arg if not present. Basically a getOrElse call. The routine already exists in the jira package but I think it is private scope, so main wont have access, so we would need/want to make that public.

-Coy

Mike Pountney added 2 commits January 23, 2016 18:06
This simplifies the interface to voting, as per the discussion in
go-jira#26

Basically, DRY out the logic into a single CmdVote function based
around an 'up' bool. Similarly, make a single CLI command with an
option to do the downvote.
@mikepea
Copy link
Collaborator Author

mikepea commented Jan 24, 2016

Hey @coryb -- here's the vote --down update as discussed. I'll push up the CmdWatch update as a separate PR.

mikepea pushed a commit to mikepea/go-jira that referenced this pull request Jan 24, 2016
This adjusts the CmdWatch interface as per discussion in
go-jira#26

It also exposes public versions of the c.getOptString and c.getOptBool
utility functions, again as discussed.

The interface to CmdWatch now includes the user to be watched (rather than
depending on the opt[] map. This makes CmdWatch more useful externally.

A '--remove' option has been created, to allow for removal of a given watcher.
This was deliberately not included in the defaults map, as it is specifically only
used for 'watch' command right now. It should be moved up to a default if it becomes
a more common option, I guess (as 'remove is false' isn't a bad default)
coryb added a commit that referenced this pull request Jan 24, 2016
@coryb coryb merged commit 3bf28d2 into go-jira:master Jan 24, 2016
@coryb
Copy link
Contributor

coryb commented Jan 24, 2016

Looks great, thanks!
-Cory

pdericson pushed a commit to pdericson/go-jira that referenced this pull request Sep 14, 2017
This simplifies the interface to voting, as per the discussion in
go-jira#26

Basically, DRY out the logic into a single CmdVote function based
around an 'up' bool. Similarly, make a single CLI command with an
option to do the downvote.
pdericson pushed a commit to pdericson/go-jira that referenced this pull request Sep 14, 2017
This adjusts the CmdWatch interface as per discussion in
go-jira#26

It also exposes public versions of the c.getOptString and c.getOptBool
utility functions, again as discussed.

The interface to CmdWatch now includes the user to be watched (rather than
depending on the opt[] map. This makes CmdWatch more useful externally.

A '--remove' option has been created, to allow for removal of a given watcher.
This was deliberately not included in the defaults map, as it is specifically only
used for 'watch' command right now. It should be moved up to a default if it becomes
a more common option, I guess (as 'remove is false' isn't a bad default)
pdericson pushed a commit to pdericson/go-jira that referenced this pull request Sep 14, 2017
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.

2 participants