Skip to content
This repository has been archived by the owner on Nov 14, 2023. It is now read-only.

jira import improvements #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

neoswallow
Copy link

• 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

neoswallow and others added 2 commits September 25, 2019 10:41
• 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
Copy link
Contributor

@jorilallo jorilallo left a 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> => {
Copy link
Contributor

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 = {
Copy link
Contributor

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 = [
Copy link
Contributor

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,
Copy link
Contributor

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)']})`
: '';
Copy link
Contributor

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):`;
Copy link
Contributor

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants