Skip to content
This repository has been archived by the owner on Mar 11, 2021. It is now read-only.

Adds WIT parameter in TrackerQuery payload #2360

Merged
merged 26 commits into from
Dec 21, 2018
Merged

Adds WIT parameter in TrackerQuery payload #2360

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
f0fc73c
Adds WIT to TQ payload
DhritiShikhar Dec 2, 2018
65b3643
Merge remote-tracking branch 'upstream/master' into tq-wit-1
DhritiShikhar Dec 2, 2018
e4f054c
adds check for WIT; adds tests
DhritiShikhar Dec 3, 2018
21ee0b5
adds require.NoError
DhritiShikhar Dec 3, 2018
bba6209
removes duplicate checks for TrackerQuery update action
DhritiShikhar Dec 3, 2018
0524fee
replaces `rest` with `s`
DhritiShikhar Dec 3, 2018
5c24211
uses app.NewSpaceRelation(....)
DhritiShikhar Dec 3, 2018
4e28161
changes `baseType` to `workItemType` in TrackerQuery payload
DhritiShikhar Dec 3, 2018
7eafcf3
removes unnecessary spacetemplate creation
DhritiShikhar Dec 3, 2018
936cfc9
uses app.NewSpaceRelation(....) for space
DhritiShikhar Dec 3, 2018
68498a6
Adds work_item_type_id to existing tracker_queries; Adds tests;
DhritiShikhar Dec 3, 2018
3646533
Merge remote-tracking branch 'upstream/master' into tq-wit-1
DhritiShikhar Dec 3, 2018
72bc5c5
Adds struct tags
DhritiShikhar Dec 3, 2018
6648376
changes name of a function
DhritiShikhar Dec 3, 2018
071fe92
removes t:=s.T() line controller level test
DhritiShikhar Dec 3, 2018
97f2247
deletes old data from tracker_queries; adds wit column in tracker_que…
DhritiShikhar Dec 4, 2018
6f48605
removes unnecessary code from migration black box tests
DhritiShikhar Dec 4, 2018
e358263
Merge branch 'master' into tq-wit-1
DhritiShikhar Dec 6, 2018
6ddb69d
renames function
DhritiShikhar Dec 10, 2018
92a0e2c
Merge remote-tracking branch 'origin/tq-wit-1' into tq-wit-1
DhritiShikhar Dec 10, 2018
f5c9755
Merge remote-tracking branch 'upstream/master' into tq-wit-1
DhritiShikhar Dec 10, 2018
fc1a8ba
Merge branch 'master' into tq-wit-1
DhritiShikhar Dec 10, 2018
1b36616
changes assert to require
DhritiShikhar Dec 17, 2018
09a0e75
adds return values for tests
DhritiShikhar Dec 17, 2018
a59a40b
puts tests under one function
DhritiShikhar Dec 17, 2018
9fd13d7
Merge remote-tracking branch 'upstream/master' into tq-wit-1
DhritiShikhar Dec 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions controller/trackerquery.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,15 @@ func (c *TrackerqueryController) Create(ctx *app.CreateTrackerqueryContext) erro
}
err = application.Transactional(c.db, func(appl application.Application) error {
trackerQuery := remoteworkitem.TrackerQuery{
Query: ctx.Payload.Data.Attributes.Query,
Schedule: ctx.Payload.Data.Attributes.Schedule,
TrackerID: ctx.Payload.Data.Relationships.Tracker.Data.ID,
SpaceID: *ctx.Payload.Data.Relationships.Space.Data.ID,
Query: ctx.Payload.Data.Attributes.Query,
Schedule: ctx.Payload.Data.Attributes.Schedule,
TrackerID: ctx.Payload.Data.Relationships.Tracker.Data.ID,
SpaceID: *ctx.Payload.Data.Relationships.Space.Data.ID,
WorkItemTypeID: ctx.Payload.Data.Relationships.BaseType.Data.ID,
}
if ctx.Payload.Data.ID != nil {
trackerQuery.ID = *ctx.Payload.Data.ID
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder whether the UUID should be validated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@baijum are you talking about a check if the id contains string or uuid?

Copy link
Contributor

Choose a reason for hiding this comment

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

Check whether the value of *ctx.Payload.Data.ID is real UUID (It's not a string type)

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean what if the value is some random string. It will reach the database layer and fail. How about failing early?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@baijum At design level itself its specified that the value should be a UUID (https://github.com/fabric8-services/fabric8-wit/blob/master/design/tracker_queries.go#L13)

}
trackerQuery.ID = *ctx.Payload.Data.ID
tq, err := appl.TrackerQueries().Create(ctx.Context, trackerQuery)
if err != nil {
return errs.Wrapf(err, "failed to create tracker query %s", ctx.Payload.Data)
Expand Down Expand Up @@ -157,6 +160,9 @@ func (c *TrackerqueryController) Update(ctx *app.UpdateTrackerqueryContext) erro
if &ctx.Payload.Data.Relationships.Tracker.Data.ID != nil {
tq.TrackerID = ctx.Payload.Data.Relationships.Tracker.Data.ID
}
if ctx.Payload.Data.Relationships.BaseType.Data.ID == uuid.Nil {
return errors.NewBadParameterError("Workitemtype_id", nil).Expected("not nil")
}
_, err = appl.TrackerQueries().Save(ctx.Context, *tq)
if err != nil {
return errs.Wrapf(err, "failed to update tracker query %s", ctx.Payload.Data.ID)
Expand Down Expand Up @@ -248,6 +254,9 @@ func validateCreateTrackerQueryPayload(ctx *app.CreateTrackerqueryContext) error
if *ctx.Payload.Data.Relationships.Space.Data.ID == uuid.Nil {
return errors.NewBadParameterError("SpaceID", nil).Expected("not nil")
}
if ctx.Payload.Data.Relationships.BaseType.Data.ID == uuid.Nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You hat the same check on line 163, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kwk the check here is for validating create action. The check on line 163 is for validating update action.

However, there are some duplicate checks for update action. Made changes here bba6209

return errors.NewBadParameterError("Workitemtype_id", nil).Expected("not nil")
}
return nil
}

Expand Down
82 changes: 62 additions & 20 deletions controller/trackerquery_blackbox_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,10 @@ func (rest *TestTrackerQueryREST) TestCreateTrackerQuery() {
resource.Require(t, resource.Database)

svc, _, trackerQueryCtrl := rest.SecuredController()
fxt := tf.NewTestFixture(t, rest.DB, tf.Spaces(1), tf.Trackers(1))
fxt := tf.NewTestFixture(t, rest.DB, tf.Spaces(1), tf.Trackers(1), tf.WorkItemTypes(1))
assert.NotNil(t, fxt.Spaces[0], fxt.Trackers[0])

tqpayload := newCreateTrackerQueryPayload(fxt.Spaces[0].ID, fxt.Trackers[0].ID)

tqpayload := newCreateTrackerQueryPayload(fxt.Spaces[0].ID, fxt.Trackers[0].ID, fxt.WorkItemTypes[0].ID)
_, tqresult := test.CreateTrackerqueryCreated(t, svc.Context, svc, trackerQueryCtrl, &tqpayload)
assert.NotNil(t, tqresult)
}
Expand All @@ -193,13 +192,12 @@ func (rest *TestTrackerQueryREST) TestShowTrackerQuery() {
resource.Require(t, resource.Database)

svc, _, trackerQueryCtrl := rest.SecuredController()
fxt := tf.NewTestFixture(t, rest.DB, tf.Spaces(1), tf.Trackers(1))
fxt := tf.NewTestFixture(t, rest.DB, tf.Spaces(1), tf.Trackers(1), tf.WorkItemTypes(1))
assert.NotNil(t, fxt.Spaces[0], fxt.Trackers[0])

tqpayload := newCreateTrackerQueryPayload(fxt.Spaces[0].ID, fxt.Trackers[0].ID)
tqpayload := newCreateTrackerQueryPayload(fxt.Spaces[0].ID, fxt.Trackers[0].ID, fxt.WorkItemTypes[0].ID)

_, tqresult := test.CreateTrackerqueryCreated(t, svc.Context, svc, trackerQueryCtrl, &tqpayload)

_, tqr := test.ShowTrackerqueryOK(t, svc.Context, svc, trackerQueryCtrl, *tqresult.Data.ID)
assert.NotNil(t, tqr)
assert.Equal(t, tqresult.Data.ID, tqr.Data.ID)
Expand All @@ -210,19 +208,17 @@ func (rest *TestTrackerQueryREST) TestUpdateTrackerQuery() {
resource.Require(t, resource.Database)

svc, _, trackerQueryCtrl := rest.SecuredController()
fxt := tf.NewTestFixture(t, rest.DB, tf.Spaces(1), tf.Trackers(1))
fxt := tf.NewTestFixture(t, rest.DB, tf.Spaces(1), tf.Trackers(1), tf.WorkItemTypes(1))
assert.NotNil(t, fxt.Spaces[0], fxt.Trackers[0])

tqpayload := newCreateTrackerQueryPayload(fxt.Spaces[0].ID, fxt.Trackers[0].ID)
tqpayload := newCreateTrackerQueryPayload(fxt.Spaces[0].ID, fxt.Trackers[0].ID, fxt.WorkItemTypes[0].ID)

_, tqresult := test.CreateTrackerqueryCreated(t, svc.Context, svc, trackerQueryCtrl, &tqpayload)

_, tqr := test.ShowTrackerqueryOK(t, svc.Context, svc, trackerQueryCtrl, *tqresult.Data.ID)
assert.NotNil(t, tqr)
assert.Equal(t, tqresult.Data.ID, tqr.Data.ID)

spaceID := space.SystemSpace
trackerID := fxt.Trackers[0].ID
payload2 := app.UpdateTrackerqueryPayload{
Data: &app.TrackerQuery{
ID: tqr.Data.ID,
Expand All @@ -233,15 +229,22 @@ func (rest *TestTrackerQueryREST) TestUpdateTrackerQuery() {
Relationships: &app.TrackerQueryRelations{
Space: &app.RelationSpaces{
kwk marked this conversation as resolved.
Show resolved Hide resolved
Data: &app.RelationSpacesData{
ID: &spaceID,
ID: &fxt.Spaces[0].ID,
Type: &space.SpaceType,
},
},
Tracker: &app.RelationKindUUID{
Data: &app.DataKindUUID{
ID: trackerID,
ID: fxt.Trackers[0].ID,
Type: remoteworkitem.APIStringTypeTrackers,
},
},
BaseType: &app.RelationBaseType{
Data: &app.BaseTypeData{
ID: fxt.WorkItemTypes[0].ID,
Type: APIStringTypeWorkItemType,
},
},
},
Type: remoteworkitem.APIStringTypeTrackerQuery,
},
Expand All @@ -260,14 +263,14 @@ func (rest *TestTrackerQueryREST) TestTrackerQueryListItemsNotNil() {
resource.Require(t, resource.Database)

svc, _, trackerQueryCtrl := rest.SecuredController()
fxt := tf.NewTestFixture(t, rest.DB, tf.Spaces(1), tf.Trackers(1))
fxt := tf.NewTestFixture(t, rest.DB, tf.Spaces(1), tf.Trackers(1), tf.WorkItemTypes(1))
assert.NotNil(t, fxt.Spaces[0], fxt.Trackers[0])

tqpayload := newCreateTrackerQueryPayload(fxt.Spaces[0].ID, fxt.Trackers[0].ID)
tqpayload := newCreateTrackerQueryPayload(fxt.Spaces[0].ID, fxt.Trackers[0].ID, fxt.WorkItemTypes[0].ID)
_, tq1 := test.CreateTrackerqueryCreated(t, svc.Context, svc, trackerQueryCtrl, &tqpayload)
assert.NotNil(t, tq1)

tqpayload2 := newCreateTrackerQueryPayload(fxt.Spaces[0].ID, fxt.Trackers[0].ID)
tqpayload2 := newCreateTrackerQueryPayload(fxt.Spaces[0].ID, fxt.Trackers[0].ID, fxt.WorkItemTypes[0].ID)
_, tq2 := test.CreateTrackerqueryCreated(t, svc.Context, svc, trackerQueryCtrl, &tqpayload2)
assert.NotNil(t, tq2)

Expand All @@ -282,10 +285,10 @@ func (rest *TestTrackerQueryREST) TestCreateTrackerQueryID() {
resource.Require(t, resource.Database)

svc, _, trackerQueryCtrl := rest.SecuredController()
fxt := tf.NewTestFixture(t, rest.DB, tf.Spaces(1), tf.Trackers(1))
fxt := tf.NewTestFixture(t, rest.DB, tf.Spaces(1), tf.Trackers(1), tf.WorkItemTypes(1))

rest.T().Run("valid - success", func(t *testing.T) {
tqpayload := newCreateTrackerQueryPayload(fxt.Spaces[0].ID, fxt.Trackers[0].ID)
tqpayload := newCreateTrackerQueryPayload(fxt.Spaces[0].ID, fxt.Trackers[0].ID, fxt.WorkItemTypes[0].ID)
_, trackerquery := test.CreateTrackerqueryCreated(t, svc.Context, svc, trackerQueryCtrl, &tqpayload)
require.NotNil(t, trackerquery)

Expand All @@ -294,14 +297,46 @@ func (rest *TestTrackerQueryREST) TestCreateTrackerQueryID() {
assert.Equal(t, trackerquery.Data.ID, result.Data.ID)
})
rest.T().Run("invalid - fail", func(t *testing.T) {
tqpayload := newCreateTrackerQueryPayload(fxt.Spaces[0].ID, fxt.Trackers[0].ID)
tqpayload := newCreateTrackerQueryPayload(fxt.Spaces[0].ID, fxt.Trackers[0].ID, fxt.WorkItemTypes[0].ID)
invalidID := uuid.Nil
tqpayload.Data.ID = &invalidID
test.CreateTrackerqueryBadRequest(t, svc.Context, svc, trackerQueryCtrl, &tqpayload)
})
}

func newCreateTrackerQueryPayload(spaceID uuid.UUID, trackerID uuid.UUID) app.CreateTrackerqueryPayload {
func (rest *TestTrackerQueryREST) TestInvalidWITinTrackerQuery() {
t := rest.T()
kwk marked this conversation as resolved.
Show resolved Hide resolved
resource.Require(t, resource.Database)
rest.T().Run("nil WIT in trackerquery payload", func(t *testing.T) {
fxt := tf.NewTestFixture(t, rest.DB,
tf.SpaceTemplates(2),
kwk marked this conversation as resolved.
Show resolved Hide resolved
tf.Spaces(1),
tf.Trackers(1),
)
svc, _, trackerQueryCtrl := rest.SecuredController()

tqpayload := newCreateTrackerQueryPayload(fxt.Spaces[0].ID, fxt.Trackers[0].ID, uuid.Nil)
test.CreateTrackerqueryBadRequest(t, svc.Context, svc, trackerQueryCtrl, &tqpayload)
})

rest.T().Run("disallow creation if WIT belongs to different spacetemplate", func(t *testing.T) {
fxt := tf.NewTestFixture(t, rest.DB,
tf.SpaceTemplates(2),
tf.Spaces(1),
tf.WorkItemTypes(1, func(fxt *tf.TestFixture, idx int) error {
fxt.WorkItemTypes[idx].SpaceTemplateID = fxt.SpaceTemplates[1].ID
return nil
}),
tf.Trackers(1),
)
svc, _, trackerQueryCtrl := rest.SecuredController()

tqpayload := newCreateTrackerQueryPayload(fxt.Spaces[0].ID, fxt.Trackers[0].ID, fxt.WorkItemTypes[0].ID)
test.CreateTrackerqueryBadRequest(t, svc.Context, svc, trackerQueryCtrl, &tqpayload)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't these test methods have return values that you can check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

09a0e75

})
}

func newCreateTrackerQueryPayload(spaceID uuid.UUID, trackerID uuid.UUID, witID uuid.UUID) app.CreateTrackerqueryPayload {
trackerQueryID := uuid.NewV4()
return app.CreateTrackerqueryPayload{
Data: &app.TrackerQuery{
Expand All @@ -313,7 +348,8 @@ func newCreateTrackerQueryPayload(spaceID uuid.UUID, trackerID uuid.UUID) app.Cr
Relationships: &app.TrackerQueryRelations{
Space: &app.RelationSpaces{
Data: &app.RelationSpacesData{
ID: &spaceID,
ID: &spaceID,
Type: &space.SpaceType,
},
},
Tracker: &app.RelationKindUUID{
Expand All @@ -322,6 +358,12 @@ func newCreateTrackerQueryPayload(spaceID uuid.UUID, trackerID uuid.UUID) app.Cr
Type: remoteworkitem.APIStringTypeTrackers,
},
},
BaseType: &app.RelationBaseType{
Data: &app.BaseTypeData{
ID: witID,
Type: APIStringTypeWorkItemType,
},
},
},
Type: remoteworkitem.APIStringTypeTrackerQuery,
},
Expand Down
1 change: 1 addition & 0 deletions design/tracker_queries.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ var trackerQueryAttributes = a.Type("TrackerQueryAttributes", func() {
var trackerQueryRelationships = a.Type("TrackerQueryRelations", func() {
a.Attribute("tracker", relationKindUUID, "This defines the related tracker")
a.Attribute("space", relationSpaces, "This defines the owning space")
a.Attribute("baseType", relationBaseType, "This defines type of Work Item")
kwk marked this conversation as resolved.
Show resolved Hide resolved
kwk marked this conversation as resolved.
Show resolved Hide resolved
})

var trackerQueryList = JSONList(
Expand Down
2 changes: 2 additions & 0 deletions migration/migration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,8 @@ func GetMigrations() Migrations {
// Version 110
m = append(m, steps{ExecuteSQLFile("110-trackerquery-to-use-uuid.sql")})

// Version 111
m = append(m, steps{ExecuteSQLFile("111-add-wit-to-trackerquery.sql")})
// Version N
//
// In order to add an upgrade, simply append an array of MigrationFunc to the
Expand Down
1 change: 1 addition & 0 deletions migration/sql-files/111-add-wit-to-trackerquery.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE tracker_queries ADD COLUMN work_item_type_id uuid REFERENCES work_item_types(id);
kwk marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion remoteworkitem/remoteworkitem.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ var RemoteWorkItemKeyMaps = map[string]RemoteWorkItemMap{
},
ProviderJira: {
AttributeMapper{AttributeExpression(JiraTitle), StringConverter{}}: remoteTitle,
AttributeMapper{AttributeExpression(JiraBody), MarkupConverter{markup: rendering.SystemMarkupJiraWiki}}: remoteDescription,
AttributeMapper{AttributeExpression(JiraBody), MarkupConverter{markup: rendering.SystemMarkupMarkdown}}: remoteDescription,
kwk marked this conversation as resolved.
Show resolved Hide resolved
AttributeMapper{AttributeExpression(JiraState), JiraStateConverter{}}: remoteState,
AttributeMapper{AttributeExpression(JiraID), StringConverter{}}: remoteItemID,
AttributeMapper{AttributeExpression(JiraCreatorLogin), StringConverter{}}: remoteCreatorLogin,
Expand Down
25 changes: 13 additions & 12 deletions remoteworkitem/scheduler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@ import (
)

// TrackerSchedule capture all configuration
type trackerSchedule struct {
TrackerID uuid.UUID
URL string
TrackerType string
Query string
Schedule string
SpaceID uuid.UUID
type TrackerSchedule struct {
kwk marked this conversation as resolved.
Show resolved Hide resolved
TrackerID uuid.UUID
URL string
TrackerType string
Query string
Schedule string
SpaceID uuid.UUID
WorkItemTypeID uuid.UUID
}

// Scheduler represents scheduler
Expand Down Expand Up @@ -67,7 +68,7 @@ func (s *Scheduler) ScheduleAllQueries(ctx context.Context, accessTokens map[str
return errors.WithStack(err)
}
// Convert the remote item into a local work item and persist in the DB.
_, err = ConvertToWorkItemModel(ctx, tx, tq.TrackerID, i, tq.TrackerType, tq.SpaceID)
_, err = ConvertToWorkItemModel(ctx, tx, i, tq)
return errors.WithStack(err)
})
}
Expand All @@ -76,9 +77,9 @@ func (s *Scheduler) ScheduleAllQueries(ctx context.Context, accessTokens map[str
cr.Start()
}

func fetchTrackerQueries(db *gorm.DB) []trackerSchedule {
tsList := []trackerSchedule{}
err := db.Table("tracker_queries").Select("trackers.id as tracker_id, trackers.url, trackers.type as tracker_type, tracker_queries.query, tracker_queries.schedule, tracker_queries.space_id").Joins("left join trackers on tracker_queries.tracker_id = trackers.id").Where("trackers.deleted_at is NULL AND tracker_queries.deleted_at is NULL").Scan(&tsList).Error
func fetchTrackerQueries(db *gorm.DB) []TrackerSchedule {
tsList := []TrackerSchedule{}
err := db.Table("tracker_queries").Select("trackers.id as tracker_id, trackers.url, trackers.type as tracker_type, tracker_queries.query, tracker_queries.schedule, tracker_queries.space_id, tracker_queries.work_item_type_id as work_item_type_id").Joins("left join trackers on tracker_queries.tracker_id = trackers.id").Where("trackers.deleted_at is NULL AND tracker_queries.deleted_at is NULL").Scan(&tsList).Error
kwk marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
log.Error(nil, map[string]interface{}{
"err": err,
Expand All @@ -88,7 +89,7 @@ func fetchTrackerQueries(db *gorm.DB) []trackerSchedule {
}

// lookupProvider provides the respective tracker based on the type
func lookupProvider(ts trackerSchedule) TrackerProvider {
func lookupProvider(ts TrackerSchedule) TrackerProvider {
switch ts.TrackerType {
case ProviderGithub:
return &GithubTracker{URL: ts.URL, Query: ts.Query}
Expand Down
6 changes: 3 additions & 3 deletions remoteworkitem/scheduler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ import (
func TestLookupProvider(t *testing.T) {
resource.Require(t, resource.UnitTest)

ts1 := trackerSchedule{TrackerType: ProviderGithub}
ts1 := TrackerSchedule{TrackerType: ProviderGithub}
tp1 := lookupProvider(ts1)
require.NotNil(t, tp1)

ts2 := trackerSchedule{TrackerType: ProviderJira}
ts2 := TrackerSchedule{TrackerType: ProviderJira}
tp2 := lookupProvider(ts2)
require.NotNil(t, tp2)

ts3 := trackerSchedule{TrackerType: "unknown"}
ts3 := TrackerSchedule{TrackerType: "unknown"}
tp3 := lookupProvider(ts3)
require.Nil(t, tp3)
}
24 changes: 12 additions & 12 deletions remoteworkitem/trackeritem_repository.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,39 +33,39 @@ func Upload(db *gorm.DB, tID uuid.UUID, item TrackerItemContent) error {
}

// Map a remote work item into an WIT work item and persist it into the database.
func ConvertToWorkItemModel(ctx context.Context, db *gorm.DB, tID uuid.UUID, item TrackerItemContent, providerType string, spaceID uuid.UUID) (*workitem.WorkItem, error) {
func ConvertToWorkItemModel(ctx context.Context, db *gorm.DB, item TrackerItemContent, tq TrackerSchedule) (*workitem.WorkItem, error) {
remoteID := item.ID
content := string(item.Content)
trackerItem := TrackerItem{Item: content, RemoteItemID: remoteID, TrackerID: tID}
trackerItem := TrackerItem{Item: content, RemoteItemID: remoteID, TrackerID: tq.TrackerID}
// Converting the remote item to a local work item
remoteTrackerItemConvertFunc, ok := RemoteWorkItemImplRegistry[providerType]
remoteTrackerItemConvertFunc, ok := RemoteWorkItemImplRegistry[tq.TrackerType]
if !ok {
return nil, BadParameterError{parameter: providerType, value: providerType}
return nil, BadParameterError{parameter: tq.TrackerType, value: tq.TrackerType}
}
remoteTrackerItem, err := remoteTrackerItemConvertFunc(trackerItem)
if err != nil {
return nil, InternalError{simpleError{message: fmt.Sprintf(" Error parsing the tracker data: %s", err.Error())}}
}
remoteWorkItem, err := Map(remoteTrackerItem, RemoteWorkItemKeyMaps[providerType])
remoteWorkItem, err := Map(remoteTrackerItem, RemoteWorkItemKeyMaps[tq.TrackerType])
if err != nil {
return nil, ConversionError{simpleError{message: fmt.Sprintf("Error mapping to local work item: %s", err.Error())}}
}
workItem, err := lookupIdentities(ctx, db, remoteWorkItem, providerType, spaceID)
workItem, err := lookupIdentities(ctx, db, remoteWorkItem, tq)
if err != nil {
return nil, InternalError{simpleError{message: fmt.Sprintf("Error bind assignees: %s", err.Error())}}
}
return upsert(ctx, db, *workItem)
}

// lookupIdentities looks up creator and assignee remote identities to local identities (already existing or to be created)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please create a better documentation line. It makes no sense to me :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The function did much more than lookupIdentities. Changed the function name here - 6648376

Copy link
Collaborator

Choose a reason for hiding this comment

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

@DhritiShikhar // setLocalWorkItemFields sets fields of local workitem isn't really a good explanation either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kwk renamed here
6ddb69d

func lookupIdentities(ctx context.Context, db *gorm.DB, remoteWorkItem RemoteWorkItem, providerType string, spaceID uuid.UUID) (*workitem.WorkItem, error) {
func lookupIdentities(ctx context.Context, db *gorm.DB, remoteWorkItem RemoteWorkItem, tq TrackerSchedule) (*workitem.WorkItem, error) {
identityRepository := account.NewIdentityRepository(db)
//spaceSelfURL := rest.AbsoluteURL(goa.ContextRequest(ctx), app.SpaceHref(spaceID.String()))
workItem := workitem.WorkItem{
// ID: remoteWorkItem.ID,
Type: remoteWorkItem.Type,
Type: tq.WorkItemTypeID,
Fields: make(map[string]interface{}),
SpaceID: spaceID,
SpaceID: tq.SpaceID,
}
// copy all fields from remoteworkitem into result workitem
for fieldName, fieldValue := range remoteWorkItem.Fields {
Expand All @@ -77,7 +77,7 @@ func lookupIdentities(ctx context.Context, db *gorm.DB, remoteWorkItem RemoteWor
}
creatorLogin := fieldValue.(string)
creatorProfileURL := remoteWorkItem.Fields[remoteCreatorProfileURL].(string)
identity, err := identityRepository.Lookup(context.Background(), creatorLogin, creatorProfileURL, providerType)
identity, err := identityRepository.Lookup(context.Background(), creatorLogin, creatorProfileURL, tq.TrackerType)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm a bit sceptical for why and how the tracker type corresponds to the provider type in the identity repo. Can you please explain what both are and if it is only by accident that you can use the tracker type for it?

if err != nil {
return nil, errors.Wrap(err, "failed to create identity during lookup")
}
Expand All @@ -97,7 +97,7 @@ func lookupIdentities(ctx context.Context, db *gorm.DB, remoteWorkItem RemoteWor
assigneeProfileURLs := remoteWorkItem.Fields[RemoteAssigneeProfileURLs].([]string)
for i, assigneeLogin := range assigneeLogins {
assigneeProfileURL := assigneeProfileURLs[i]
identity, err := identityRepository.Lookup(context.Background(), assigneeLogin, assigneeProfileURL, providerType)
identity, err := identityRepository.Lookup(context.Background(), assigneeLogin, assigneeProfileURL, tq.TrackerType)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -149,7 +149,7 @@ func upsert(ctx context.Context, db *gorm.DB, workItem workitem.WorkItem) (*work
}
} else {
log.Info(nil, nil, "Workitem does not exist, will be created")
resultWorkItem, _, err = wir.Create(ctx, workItem.SpaceID, workitem.SystemBug, workItem.Fields, creator)
resultWorkItem, _, err = wir.Create(ctx, workItem.SpaceID, workItem.Type, workItem.Fields, creator)
if err != nil {
return nil, errors.WithStack(err)
}
Expand Down
Loading