Allow updating a task's status to RESOLVED or OPEN #188

ftclausen · 2019-03-21T23:27:37Z

closes #175

This is my first take at allowing a task's status to be changed after it has been made base on our discussions in the issue above. Not sure if it is what you had in mind; we can definitely change it as required.

Some notes

I created an UpdateTask class instead of CreateTaskUpdate because it seemed a less confusing name
Added mock tests to open task, resolve task and testing the error path
Added live test (against local Bitbucket instance using dev license). This is just one test that both opens and resolves a task. Seems easier than messing with test dependencies.
I could not get the live tests working with HTTP endpoints using just a username so modified TestUtilities to send the full user:pass string
Also modified TestUtilities to do a case insensitive comparison when trying to find the Bitbucket user

I'll comment some more inline.

And cleanup some notes

Also make slug comparisons case insenstive

- Make transient task for check/uncheck test because you can't delete resolved tasks - Update task deletion test to original state - Rework live test to make more sense - Add mock tests for opening and resolving a task

src/test/java/com/cdancy/bitbucket/rest/features/TasksApiLiveTest.java

src/test/java/com/cdancy/bitbucket/rest/features/TasksApiMockTest.java

src/test/java/com/cdancy/bitbucket/rest/features/CommentsApiLiveTest.java

src/test/java/com/cdancy/bitbucket/rest/features/PullRequestApiLiveTest.java

cdancy · 2019-03-22T01:08:40Z

I created an UpdateTask class instead of CreateTaskUpdate because it seemed a less confusing name

FWIW: I only kept this naming convention because that's what jclouds uses and I wanted any folks familiar with that framework to be able to jump in here and hit the ground running. I personally don't care for the convention at all just did it for standards sake. With that in mind: I'm OK with this name :)

src/main/java/com/cdancy/bitbucket/rest/options/UpdateTask.java

… that file

…e it This reverts commit 0ce779b.

README.md

Without the Anchor we need fewer calls

…al UpdateTask class Now it is much simpler just to pass a string of the desired state and set the taskId in the URL

Intellij Idea likes to collapse down imports if there are many from the same package but PMD does not like this. So I unexpand them but then forgot to add the actual import.

src/main/java/com/cdancy/bitbucket/rest/features/TasksApi.java

cdancy · 2019-03-27T14:01:14Z

src/test/java/com/cdancy/bitbucket/rest/features/TasksApiLiveTest.java

+        assertThat(instanceReopened.errors().isEmpty()).isTrue();
+        assertThat(instanceReopened.state()).isEqualTo("OPEN");
+    }
+


Lets also add another test to check when things fail. Maybe pass in a bogus taskId and ensure that the errors() object gets properly populated.

I have added an on failure test in 4ba5d22

cdancy · 2019-03-27T14:01:33Z

Looks good. Couple more comments to address and I think we should be good to go.

ftclausen · 2019-03-28T05:00:00Z

Looks good. Couple more comments to address and I think we should be good to go.

I think have addressed the outstanding items but, as you might expect, I am used to working in Bitbucket so I may have missed something. Let me know if that is the case.

Once everything is looking OK I'll cleanup the feature branch history.

cdancy · 2019-03-28T18:17:55Z

@ftclausen everything LGTM. Going to run the integTests in a bit to confirm all works as expected but code-wise everything is good. As far as "cleaning up the branch and/or squashing" I've been not wanting to do that as of late. I've always like the idea, and have done so in my projects, but it removes the ability to be able to cherry-pick out and/or revert single commits. Does make the history look very clean but at the cost of being able to pin-point very specific commits. I would say leave things as-is and if all goes well with testing here I'll merge later today.

ftclausen · 2019-03-29T00:49:18Z

@ftclausen everything LGTM. Going to run the integTests in a bit to confirm all works as expected but code-wise everything is good. As far as "cleaning up the branch and/or squashing" I've been not wanting to do that as of late. I've always like the idea, and have done so in my projects, but it removes the ability to be able to cherry-pick out and/or revert single commits. Does make the history look very clean but at the cost of being able to pin-point very specific commits. I would say leave things as-is and if all goes well with testing here I'll merge later today.

No worries about rewriting branch history - I'm fine to leave it as is. My integTest runs have been OK on a development instance of Bitbucket server 5.16.3 (not tried 6 yet).

cdancy · 2019-04-02T12:00:44Z

@ftclausen changes have been merged to master. I did have to revert your changes to TestUtilities as they were breaking for me. Otherwise everything else looks good.

cdancy · 2019-04-02T12:04:20Z

Version 2.5.2 has been released.

ftclausen · 2019-04-11T05:46:19Z

@ftclausen changes have been merged to master. I did have to revert your changes to TestUtilities as they were breaking for me. Otherwise everything else looks good.

Great news, and no problem at all for reverting my changes. I'll figure out what was going on on my end when I get to the next contribution (fetching all tasks for a PR).

cdancy · 2019-04-11T14:21:44Z

@ftclausen sounds good.

ftclausen added 12 commits March 22, 2019 10:11

Modify minimal repo to also include repo ID

0ce779b

Add new "CreateTaskUpdate" class to represent a Task update request

6448f85

Add update method for TasksApi

18b8bc5

Add ID to MinimalRepository.create calls

c7a3119

Add a test to exercise checking/unchecking a task

0431fbd

We need to specify taskId in the PathParam

07b1ab2

And cleanup some notes

Checkstyle fixes

43e2f13

Use complete user:pass string instead of just username

7698957

Also make slug comparisons case insenstive

Rename CreateTaskUpdate to UpdateTask which is less confusing

76831aa

Some test improvements

c37b5cc

- Make transient task for check/uncheck test because you can't delete resolved tasks - Update task deletion test to original state - Rework live test to make more sense - Add mock tests for opening and resolving a task

Checkstyle and PMD fixes

aa1a0a5

Use proper TaskAnchor instead of MinimalAnchor

73e018b

ftclausen commented Mar 21, 2019

View reviewed changes

src/test/java/com/cdancy/bitbucket/rest/features/TasksApiLiveTest.java Outdated Show resolved Hide resolved

ftclausen commented Mar 21, 2019

View reviewed changes

src/test/java/com/cdancy/bitbucket/rest/features/TasksApiLiveTest.java Outdated Show resolved Hide resolved

ftclausen commented Mar 21, 2019

View reviewed changes

src/test/java/com/cdancy/bitbucket/rest/features/TasksApiMockTest.java Outdated Show resolved Hide resolved

ftclausen commented Mar 21, 2019

View reviewed changes

src/test/java/com/cdancy/bitbucket/rest/features/TasksApiMockTest.java Outdated Show resolved Hide resolved

cdancy self-assigned this Mar 22, 2019

cdancy added the enhancement label Mar 22, 2019

cdancy added this to the v2.5.2 milestone Mar 22, 2019

cdancy reviewed Mar 22, 2019

View reviewed changes

src/test/java/com/cdancy/bitbucket/rest/features/CommentsApiLiveTest.java Outdated Show resolved Hide resolved

cdancy reviewed Mar 22, 2019

View reviewed changes

src/test/java/com/cdancy/bitbucket/rest/features/PullRequestApiLiveTest.java Outdated Show resolved Hide resolved

cdancy reviewed Mar 22, 2019

View reviewed changes

src/main/java/com/cdancy/bitbucket/rest/options/UpdateTask.java Outdated Show resolved Hide resolved

No need to declare constants for strings only used once

019a111

ftclausen force-pushed the issue/175 branch from b4ef55e to 019a111 Compare March 25, 2019 03:36

ftclausen added 3 commits March 25, 2019 16:51

Remove extra fields not required for UpdateTask

1cb90aa

Add a note about overriding gradle.properties without needing to edit…

3a34d05

… that file

We're not using MinimalRespository for UpdateTask so no need to chang…

d4c9bc9

…e it This reverts commit 0ce779b.

cdancy reviewed Mar 25, 2019

View reviewed changes

README.md Show resolved Hide resolved

Remove the requirement for needing a repo ID

075a26a

ftclausen added 3 commits March 27, 2019 13:33

Remove Anchor from API call and update Mock tests

0fe7a82

Without the Anchor we need fewer calls

With the API simplified attempt suggestion in PR to just remove speci…

007664f

…al UpdateTask class Now it is much simpler just to pass a string of the desired state and set the taskId in the URL

Fix imports and make PMD happy

9c7de4a

Intellij Idea likes to collapse down imports if there are many from the same package but PMD does not like this. So I unexpand them but then forgot to add the actual import.

cdancy reviewed Mar 27, 2019

View reviewed changes

src/main/java/com/cdancy/bitbucket/rest/features/TasksApi.java Show resolved Hide resolved

cdancy reviewed Mar 27, 2019

View reviewed changes

Add on failure test

4ba5d22

cdancy merged commit 4ba5d22 into cdancy:master Apr 2, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow updating a task's status to RESOLVED or OPEN #188

Allow updating a task's status to RESOLVED or OPEN #188

ftclausen commented Mar 21, 2019

cdancy commented Mar 22, 2019 •

edited

Loading

cdancy Mar 27, 2019

ftclausen Mar 28, 2019

cdancy commented Mar 27, 2019

ftclausen commented Mar 28, 2019

cdancy commented Mar 28, 2019

ftclausen commented Mar 29, 2019

cdancy commented Apr 2, 2019

cdancy commented Apr 2, 2019

ftclausen commented Apr 11, 2019

cdancy commented Apr 11, 2019

Allow updating a task's status to RESOLVED or OPEN #188

Allow updating a task's status to RESOLVED or OPEN #188

Conversation

ftclausen commented Mar 21, 2019

cdancy commented Mar 22, 2019 • edited Loading

cdancy Mar 27, 2019

Choose a reason for hiding this comment

ftclausen Mar 28, 2019

Choose a reason for hiding this comment

cdancy commented Mar 27, 2019

ftclausen commented Mar 28, 2019

cdancy commented Mar 28, 2019

ftclausen commented Mar 29, 2019

cdancy commented Apr 2, 2019

cdancy commented Apr 2, 2019

ftclausen commented Apr 11, 2019

cdancy commented Apr 11, 2019

cdancy commented Mar 22, 2019 •

edited

Loading