-
Notifications
You must be signed in to change notification settings - Fork 19
jira import improvements #5
base: master
Are you sure you want to change the base?
Conversation
• added yarn cli-debug for debugging on port 9500 • will modify csv header to import multiple columns of same name as an array (for attachments and comments) • add attachment as a link on description • fetch comments • option to add jira issue key as a prefix to the title
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution, much needed! I made some comments, mainly on style to make the code easier to read and follow.
Would you have an example export to include in a separate folder? Without your personal data of course but it would be useful for future as we probably want to add tests
* Assumption: same name headers are grouped together | ||
* @returns {Promise<HeaderResult>} | ||
*/ | ||
public checkHeader = (): Promise<HeaderResult> => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like better return value would be just to return the newly parsed header, or undefined, and reject
if there was an error: parseHeader(): Promise<string[] | undefined>
. This way you don't need to pass errors isn't used and you can infer HeaderChanged
from existence of the data.
public import = async (): Promise<ImportResult> => { | ||
const data = (await csv().fromFile(this.filePath)) as JiraIssueType[]; | ||
const labelColors: labelColor = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: would move to be a constant along side the importer class, not inside the method.
const statuses = Array.from(new Set(data.map(row => row['Status']))); | ||
const assignees = Array.from(new Set(data.map(row => row['Assignee']))); | ||
const assignees = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renaming to users
would make more sense in this context
type: 'confirm', | ||
name: 'includeIssueKeyInTheTitle', | ||
message: 'Include existing Jira issue key in the title (as prefix)?: ', | ||
default: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would change this to false
so there's it's more explicit decision
? `${mdDesc && '\n\n'}[View epic link in Jira \[${ | ||
row['Custom field (Epic Link)'] | ||
}\]](${baseUrl}/${row['Custom field (Epic Link)']})` | ||
: ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be written in easier to read form:
if (row['Custom field (Epic Link)'] && baseUrl) {
mdDesc += `${mdDesc}\n\n[View epic link in Jira \[${
row['Custom field (Epic Link)']
}\]](${baseUrl}/${row['Custom field (Epic Link)']})`
}
jiraAttachments = [jiraAttachments]; | ||
} | ||
if (jiraAttachments.length) { | ||
mdDesc += `${mdDesc && '\n\n'}Attachment(s):`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of always checking for mdDesc existence, maybe just all || ''
to line 186
• added yarn cli-debug for debugging on port 9500
• will modify csv header to import multiple columns of same name as an array (for attachments and comments)
• add attachment as a link on description
• fetch comments
• option to add jira issue key as a prefix to the title