Adds JIRA Integration #158

CharlesWinter · 2021-07-04T21:10:22Z

Adds a "push" command that will push your timetrace records for that day to a remote system (JIRA in this PR).

This PR is still a draft. I've thrown caution to the wind and have exactly zero tests for what I've done, as I started this work just for fun. Please don't merge it yet until there are tests. The purpose of the PR is simply to make you guys aware that I've been working on this, and to of course allow others to improve upon it as they see fit. As it stands the code does work, and records are pushed where timetrace sees fit, and are pushed only once. It of course also needs a readme entry on how to actually set it up.

How does it work? (see the GIF for a demo)

The user adds their credentials (including an API token) to the timetrace config file as specified here
Through a new cobra command "push" the user can select which integration they wish to push to (only jira at the moment)
I examine all records from today here and loop through to find a subset of those records that should be added to JIRA as worklogs, these worklogs are presented to the user to "confirm" the push.
records will be pushed to JIRA if:
A. The project ID for the record matches a ticket. e.g. tw-20@timetrace or tw-2 would be pushed to tw-20 and tw-2 respectively.
B. The record has not been pushed before.
Once confirmed, those worklogs are added to JIRA.
worklogs have metadata (entity properties in JIRA speak) in the form of a key value pair added to them of the form timetrace_record_id: "2020-10-20T10:32:00". It seems timetrace uses the startdate as a unique ID for a record so I thought I would continue the convention in JIRA. This metadata is added so a record can only be pushed once.

Technical Considerations for Reviewers

Timetrace seems to be well decoupled, with a nice use of dependency injection and inversion. I've tried to continue this with a slightly presumptious introduction of an interface here Any integration need only fufill this to be used with the push command.
The JIRA API is horrendous in that one must make a second call to add metadata to an object. This has resulted in a fair few API requests going over the wire for each push. A lot of the request making boilerplate in the repository can be consolidated into functions.
I've added a spicy terminal hack that has those cool little crosses/ticks appear in place in the table when performing a push. I've no idea if this will work on a wide variety of terminal emulators, and when it goes wrong it looks bloody awful with a feast of emojis on the screen. I'm happy to drop this, I did it more for my own personal interest, but it's pretty evil.

To conclude, still early days for this PR, as it started mainly for my own enjoyment and the code needs some love/tests/rebasing to get it in :) I'll continue working on it when I get time.

Contributions welcome.

- An initial call is made to create the worklog in JIRA - A second call is made to add a "JIRA property" that marks the record as been created by timetrace

preRunE wasn't actually too useful as it didn't cover the natural case where a user may select "no" from the option to push records. This isn't an error.

dominikbraun · 2021-07-05T07:43:30Z

Wow, thanks a lot for this massive feature! I'm just going to release timetrace v0.11.1 today and review and merge some PRs in the pipeline for timetrace v0.12.0 - then I'll be able to take a closer look at the push command.

CharlesWinter · 2021-07-05T08:14:35Z

core/provider.go

+
+// Provider is a slightly presumptious interface for how third party
+// integrations might be used. The idea is that any integration need only
+// fufill the two methods defined here, and it can be called via the push


3 not 2 methods.

CharlesWinter · 2021-07-05T08:15:12Z

core/provider_sync.go

@@ -0,0 +1 @@
+package core


delete this file.

CharlesWinter · 2021-07-05T08:21:06Z

core/timetrace.go

+// VerifyPush confirms with the user the records that are about to be uploaded
+// to the selected integ
+func (t *Timetrace) VerifyPush(integrationName string) ([]*Record, error) {
+	// get all records locally TODO: obviously improve the time parsing


This can be improved to have the user pass the "today" or "yesterday" etc as a flag. I hardcoded to today just for convenience.

CharlesWinter · 2021-07-05T08:22:37Z

integrations/jira/check_records_exist.go

+	url := fmt.Sprintf("https://%s/%s/issue/%s/worklog/%s/properties/timetrace_id",
+		r.jiraAddress,
+		jiraV3BaseURL,
+		issueName,
+		worklogID,
+	)
+	request, err := http.NewRequest("GET", url, nil)
+	if err != nil {
+		return nil, err
+	}
+	request.SetBasicAuth(r.email, r.authToken)
+
+	response, err := r.client.Do(request)
+	if err != nil {
+		return nil, err
+	}


This boilerplate is repeated across a lot of requests in the package. Extract to common function.

CharlesWinter · 2021-07-05T08:25:32Z

main.go

+	var jiraRepo *jira.Repository
+	if c.JIRAIntegration != (config.JIRAConfig{}) {
+		jiraRepo = jira.New(jira.RepositoryConfig{
+			AuthToken:   c.JIRAIntegration.APIToken,
+			Email:       c.JIRAIntegration.UserEmail,
+			JIRAAddress: c.JIRAIntegration.Host,
+		})
+	}
+
 	filesystem := fs.New(c)
-	timetrace := core.New(c, filesystem)
+	timetrace := core.New(c, filesystem, []core.Provider{jiraRepo})


I'm not actually sure this is correct on review; if the jiraRepo is nil, it likely shouldn't be passed into the core at all.

A simple test that checks the method makes 2 calls to JIRA. One with the inital worklog create and a second with the metadata to make sure it's not recreated by timetrace

dominikbraun · 2021-08-01T12:13:07Z

Hey @CharlesWinter, is this ready to be reviewed? :-)

CharlesWinter · 2021-08-02T08:14:03Z

Hi @dominikbraun it's definitely missing some tests, so I don't think it's ready for merge yet.

But I think the implementation isn't likely to change so if you want to take a look I would welcome it!

CharlesWinter added 9 commits June 23, 2021 23:06

initial commit

90a1736

Adds nice checkboxes on pushing to jira success

22ff28e

Returns correctly on error and uses orange circle for waiting

269aa81

initial pass at checking records exist in jira

237a2e6

Adds method to upload logs to JIRA

6668264

- An initial call is made to create the worklog in JIRA - A second call is made to add a "JIRA property" that marks the record as been created by timetrace

Deletes curl testing bodies for JIRA integration

4891b0d

Removes the jira-config file and cleans it from gitignore

c91b4b1

Removes the PreRunE from the pushCommand

4e574bb

preRunE wasn't actually too useful as it didn't cover the natural case where a user may select "no" from the option to push records. This isn't an error.

Merge branch 'main' into jira-integration

622a827

CharlesWinter commented Jul 5, 2021

View reviewed changes

core/provider_sync.go

@@ -0,0 +1 @@

package core

Copy link

Author

CharlesWinter Jul 5, 2021

Choose a reason for hiding this comment

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

delete this file.

CharlesWinter commented Jul 5, 2021

View reviewed changes

CharlesWinter added 3 commits July 7, 2021 19:29

Merge branch 'main' into jira-integration

7973cea

Adds initial test for upload_record to JIRA

8fc16d5

A simple test that checks the method makes 2 calls to JIRA. One with the inital worklog create and a second with the metadata to make sure it's not recreated by timetrace

Adds test to upload record to JIRA method to ensure error is returned

8f37e1a

dominikbraun added this to the timetrace v0.16.0 milestone Dec 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds JIRA Integration #158

Adds JIRA Integration #158

CharlesWinter commented Jul 4, 2021 •

edited

dominikbraun commented Jul 5, 2021

CharlesWinter Jul 5, 2021

CharlesWinter Jul 5, 2021

CharlesWinter Jul 5, 2021

CharlesWinter Jul 5, 2021

CharlesWinter Jul 5, 2021

dominikbraun commented Aug 1, 2021

CharlesWinter commented Aug 2, 2021

Adds JIRA Integration #158

Are you sure you want to change the base?

Adds JIRA Integration #158

Conversation

CharlesWinter commented Jul 4, 2021 • edited

How does it work? (see the GIF for a demo)

Technical Considerations for Reviewers

dominikbraun commented Jul 5, 2021

CharlesWinter Jul 5, 2021

Choose a reason for hiding this comment

CharlesWinter Jul 5, 2021

Choose a reason for hiding this comment

CharlesWinter Jul 5, 2021

Choose a reason for hiding this comment

CharlesWinter Jul 5, 2021

Choose a reason for hiding this comment

CharlesWinter Jul 5, 2021

Choose a reason for hiding this comment

dominikbraun commented Aug 1, 2021

CharlesWinter commented Aug 2, 2021

CharlesWinter commented Jul 4, 2021 •

edited