Skip to content

fix bug #2550#2587

Merged
mindlesscloud merged 7 commits into
apache:mainfrom
jinzhu2002:main
Aug 12, 2022
Merged

fix bug #2550#2587
mindlesscloud merged 7 commits into
apache:mainfrom
jinzhu2002:main

Conversation

@jinzhu2002
Copy link
Copy Markdown
Contributor

⚠️   Pre Checklist

Please complete ALL items in this checklist, and remove before submitting

  • I have read through the Contributing Documentation & PR Template
  • This PR is using a label (bug, feature etc.)
  • My code is has necessary documentation (if appropriate)
  • I have added any relevant tests
  • This section (⚠️   Pre Checklist) will be removed when submitting PR

Summary

Does this close any open issues?

Please mention the issues here.

Screenshots

Include any relevant screenshots here.

Other Information

Any other information that is important to this PR.

Comment thread plugins/jira/tasks/apiv2models/user.go Outdated
if u.AccountId != "" {
return u.AccountId
}
//TODO add this code,run success,no test for jira cloud
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why is this TODO?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Verify that the cloud version is OK

Comment thread plugins/jira/tasks/apiv2models/user.go Outdated
}
//TODO add this code,run success,no test for jira cloud
if u.Name != "" {
return u.Name
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The accountId is a primary key of table _tool_jira_accounts, it should be unique. How about we leave it alone and modify the Collector CollectAccounts. For the server version Jira, the endpoint api/2/user/search should be invoked instead of api/2/user which only accepts a username, not an email. Please be cautious about the response of the API api/2/user/search, which is an array, not an object. So the ResponseParser should also be adjusted accordingly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OK , I'll try to modify CollectUsers

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is one difference between the response of the two endpoints. For api/2/user, it is an object, however, the response of api/2/user/search is an array. The collector's ResponseParser should be modified to handle both cases. The collector returns a slice of interface []interface{}, and every item of the slice is a user object. There is no need to change the extractor for it always receives a single user object.

@jinzhu2002 jinzhu2002 closed this Aug 5, 2022
@jinzhu2002 jinzhu2002 reopened this Aug 5, 2022
Copy link
Copy Markdown
Contributor

@mindlesscloud mindlesscloud left a comment

Choose a reason for hiding this comment

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

LGTM

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.

3 participants