Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support children via GitHub Taskslist #315

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
60 changes: 57 additions & 3 deletions User Guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ The fundamental unit of a roadmap is a project milestone. In the context of road
- Roadmaps are represented by a single root node, or GitHub issue, which contains links to milestones contained within that roadmap.
- The roadmap root node and child milestones can be in any public repository as long as the issues satisfy the requirements outlined in this document.
- This means that you can link to existing GitHub issues as child milestones.
- Errors will be logged and displayed for the user in starmap.site

### Milestone Encodings

Expand Down Expand Up @@ -93,18 +94,38 @@ can expect to get from this milestone.
#### Children

- A milestone may have child milestones.
- Child milestones are simply full URL links to other GitHub milestone issues.
- Child milestones can exist in any public Github repository.
- Child milestones are simply GitHub issue identifiers (#<issue_number>, <org>/<repo>#<issue_number>, or full URLs) to other GitHub milestone issues.
- Child milestones can exist in any public GitHub repository.
- It is expected that child milestone issues are themselves properly encoded milestones; otherwise they will be ignored by Starmap.
- Within a parent issue, child milestones are encoded as follows (raw Markdown):

##### Tasklist syntax

Tasklists allow for "taskifying" of strings, and we have no way to link a random string to a GitHub issue. You must convert any [tasks to issues](https://docs.github.com/en/issues/tracking-your-work-with-issues/about-tasklists#converting-draft-issues-to-issues-in-a-tasklist) for them to show up as a child milestone.

We will do our best to support the expected syntax of GitHub's tasklist functionality.

See https://docs.github.com/en/issues/tracking-your-work-with-issues/about-tasklists#creating-tasklists and https://github.com/pln-planning-tools/Starmap/issues/245 for more details.

```
```[tasklist]
### Tasks
- [ ] https://github.com/pln-roadmap/Roadmap-Vizualizer/issues/10
- [ ] https://github.com/pln-roadmap/Roadmap-Vizualizer/issues/9
- [ ] https://github.com/pln-roadmap/Roadmap-Vizualizer/issues/8
\```
```

##### "Children:" syntax

This syntax is deprecated. Please see https://github.com/pln-planning-tools/Starmap/issues/245 for more details.

```
Children:
- https://github.com/pln-roadmap/Roadmap-Vizualizer/issues/10
- https://github.com/pln-roadmap/Roadmap-Vizualizer/issues/9
- https://github.com/pln-roadmap/Roadmap-Vizualizer/issues/8
```
- Errors will be logged and displayed for the user in starmap.site

### Progress Indicators

Expand All @@ -127,6 +148,39 @@ Children:
### Templates

#### Root Node Issue

##### Using GitHub Tasklists

```
Title: [Team/Project Name] [Duration] Roadmap

Description (optional):
The goal of this roadmap is to outline the key milestones and deliverables for our team/project over the next [Duration].

```[tasklist]
### Any descriptor or other text
- [ ] #123 <!-- will be recognized by starmap -->
- [ ] org/repo#123 <!-- will be recognized by starmap -->
- [ ] some non-link description <!-- will NOT be recognized by starmap -->
- [ ] https://github.com/org/repo/issue/987 <!-- will be recognized by starmap -->

### Any text
- [ ] #456 <!-- will be recognized by starmap -->
- [ ] org/repo#567 <!-- will be recognized by starmap -->
- [ ] https://github.com/other-org/other-repo/issue/987 <!-- will be recognized by starmap -->

\```

Note: This roadmap is subject to change as priorities and circumstances evolve.

Starmap Link: [Starmap Link]

```

##### Using "Children:"

**NOTE:** The children: section is deprecated. Please see https://github.com/pln-planning-tools/Starmap/issues/245 for more details

```
Title: [Team/Project Name] [Duration] Roadmap

Expand Down
2 changes: 1 addition & 1 deletion lib/backend/getGithubIssueDataWithGroupAndChildren.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { resolveChildren } from './resolveChildren';
import { resolveChildrenWithDepth } from './resolveChildrenWithDepth';

export async function getGithubIssueDataWithGroupAndChildren (issueData: GithubIssueDataWithGroup, errorManager: ErrorManager, usePendingChildren = false): Promise<GithubIssueDataWithGroupAndChildren> {
const childrenParsed: ParserGetChildrenResponse[] = getChildren(issueData.body_html);
const childrenParsed: ParserGetChildrenResponse[] = getChildren(issueData);
let pendingChildren: PendingChildren[] | undefined = undefined;
let children: GithubIssueDataWithGroupAndChildren[] = [];

Expand Down
1 change: 1 addition & 0 deletions lib/backend/issue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export async function getIssue ({ owner, repo, issue_number }): Promise<GithubIs
state: data.state as IssueStates,
node_id: data.node_id,
body_html: data.body_html || '',
body: data.body || '',
labels: data.labels
.map((label) => (typeof label !== 'string' ? label.name : label)) as string[],
};
Expand Down
109 changes: 106 additions & 3 deletions lib/parser.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { parseHTML } from 'linkedom';
import { ErrorManager } from './backend/errorManager';
import { getValidUrlFromInput } from './getValidUrlFromInput';
import { getEtaDate, isValidChildren } from './helpers';
import { GithubIssueDataWithChildren, ParserGetChildrenResponse } from './types';
import { paramsFromUrl } from './paramsFromUrl';
import { GithubIssueData, GithubIssueDataWithChildren, ParserGetChildrenResponse } from './types';

export const getDueDate = (issue: Pick<GithubIssueDataWithChildren, 'html_url' | 'body_html' | 'root_issue' | 'title'>, errorManager: ErrorManager) => {
const { body_html: issueBodyHtml } = issue;
Expand All @@ -27,8 +29,109 @@ export const getDueDate = (issue: Pick<GithubIssueDataWithChildren, 'html_url' |
};
};

export const getChildren = (issue: string): ParserGetChildrenResponse[] => {
const { document } = parseHTML(issue);
function getSectionLines(text: string, sectionHeader: string) {
const sectionIndex = text.indexOf(sectionHeader);
if (sectionIndex === -1) {
return [];
}
const lines = text.substring(sectionIndex).split(/[\r\n]+/).slice(1);
return lines;
}

const splitAndGetLastItem = (line: string) => line.trim().split(' ').slice(-1)[0]
const ensureTaskListChild = (line: string) => line.trim().indexOf('-') === 0

function getUrlStringForChildrenLine(line: string, issue: Pick<GithubIssueData, 'html_url'>) {
if (/^#\d+$/.test(line)) {
SgtPooki marked this conversation as resolved.
Show resolved Hide resolved
const { owner, repo } = paramsFromUrl(issue.html_url)
line = `${owner}/${repo}${line}`
}
return getValidUrlFromInput(line).href
}
/**
* We attempt to parse the issue.body for children included in 'tasklist' format
* @see https://github.com/pln-planning-tools/Starmap/issues/245
*
* @param {string} issue_body
*/
function getChildrenFromTaskList(issue: Pick<GithubIssueData, 'body' | 'html_url'>): ParserGetChildrenResponse[] {
// tasklists require the checkbox style format to recognize children
const lines = getSectionLines(issue.body, '```[tasklist]')
.filter(ensureTaskListChild)
.map(splitAndGetLastItem)
.filter(Boolean);

if (lines.length === 0) {
throw new Error('Section missing or has no children')
}

const children: ParserGetChildrenResponse[] = lines.map((line) => ({
group: 'tasklist',
html_url: getUrlStringForChildrenLine(line, issue),
}));
return children
}

/**
* A new version of getchildren which parses the issue body_text instead of issue body_html
*
* This function must support recognizing the current issue's organization and repo, because some children may simply be "#43" instead of a github short-id such as "org/repo#43"
* @param {string} issue
*/
function getChildrenNew(issue: Pick<GithubIssueData, 'body' | 'html_url'>): ParserGetChildrenResponse[] {
Copy link

Choose a reason for hiding this comment

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

Function getChildrenNew has a Cognitive Complexity of 13 (exceeds 5 allowed). Consider refactoring.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

your cognitive complexity is so small, an ant couldn't see it.

Copy link

Choose a reason for hiding this comment

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

Function getChildrenNew has 31 lines of code (exceeds 25 allowed). Consider refactoring.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

your lines of code are so large, their gravitational pull knocked me off my chair.

SgtPooki marked this conversation as resolved.
Show resolved Hide resolved

try {
return getChildrenFromTaskList(issue);
} catch (e) {
// Could not find children using new tasklist format,
// try to look for "children:" section
}
const lines = getSectionLines(issue.body, 'children:').map((line) => line.trim().split(' ').slice(-1)[0]).filter(Boolean);
if (lines.length === 0) {
throw new Error('Section missing or has no children')
}

// guard against HTML tags (covers cases where this method is called with issue.body_html instead of issue.body_text)
if (lines.some((line) => line.startsWith('<'))) {
throw new Error('HTML tags found in body_text');
}

const children: ParserGetChildrenResponse[] = []

for (let i = 0; i < lines.length; i++) {
const currentLine = lines[i]
if (currentLine.length <= 0) {
if (children.length === 0) {
// skip empty lines between children header and children
continue
} else {
// end of children if empty line is found and children is not empty
break
}
}

try {
children.push({
group: 'children:',
html_url: getUrlStringForChildrenLine(currentLine, issue)
})
} catch {
// invalid URL or child issue identifier, exit and return what we have
break
}
}

return children

}

export const getChildren = (issue: Pick<GithubIssueData, 'body_html' | 'body' | 'html_url'>): ParserGetChildrenResponse[] => {
try {
return getChildrenNew(issue);
} catch (e) {
// ignore failures for now, fallback to old method.
}
const { document } = parseHTML(issue.body_html);
const ulLists = [...document.querySelectorAll('ul')];
const filterListByTitle = (ulLists) =>
ulLists.filter((list) => {
Expand Down
1 change: 1 addition & 0 deletions lib/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { RoadmapMode, IssueStates, DateGranularityState } from './enums'

export interface GithubIssueData {
body_html: string;
body: string;
html_url: string;
labels: string[];
node_id: string;
Expand Down
2 changes: 1 addition & 1 deletion pages/api/roadmap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export default async function handler(
try {
const rootIssue = await getIssue({ owner, repo, issue_number });

const childrenFromBodyHtml = (!!rootIssue && rootIssue.body_html && getChildren(rootIssue.body_html)) || null;
const childrenFromBodyHtml = (!!rootIssue && rootIssue.body_html && getChildren(rootIssue)) || null;
SgtPooki marked this conversation as resolved.
Show resolved Hide resolved
let children: Awaited<ReturnType<typeof resolveChildrenWithDepth>> = [];
try {
if (childrenFromBodyHtml != null) {
Expand Down
Loading