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

Implement embeds by issueId #329

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Implement embeds by issueId #329

wants to merge 5 commits into from

Conversation

zuuring
Copy link
Member

@zuuring zuuring commented Nov 26, 2024

Notes

This implements a new feature to embed linear links based off of the issueId if it's placed in a message, such as THESIS-123.

Additionally this adds a function to delete the embed if the original message link is deleted / changed.

Still need to implement editing the original embed if it changes.

### Notes
This implements a new feature to embed linear links based off of the `issueId` if it's placed in a message, such as `THESIS-123`.

Additionally this adds a function to delete the embed if the original message link is deleted / changed.

Still need to implement editing the original embed if it changes.
This will now pull in the issue tags dynamically from the Linear API rather than having to define it in `ISSUE_PREFIXES`
Let's switch the `robot.logger.info` into `robot.logger.debug`
This adds support for editing the embeds if the original message is updated with changed issues. Ready to roll out for testing.

Adds support to `issueTagRegex` for different cased issues like `iSsUe-111` or `ISSUE-111`
@zuuring zuuring marked this pull request as ready for review November 29, 2024 12:14
@zuuring
Copy link
Member Author

zuuring commented Nov 29, 2024

Calling this one ready for review/merge. There is likely some edge cases to resolve but I suppose we can iterate further! I've implemented editing embeds and also grabbing issue ISSUE-111 prefixes automatically from the LinearSDK. So you can load an issue by simply typing in your message iSsUe-111 ISSUE-111 issue-111

// let us also track sent embeds to delete them if the original message is deleted or edited WIP
const sentEmbeds = new Map<string, Message>()

let issueTagRegex: RegExp | null = null
Copy link
Contributor

@Shadowfiend Shadowfiend Dec 3, 2024

Choose a reason for hiding this comment

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

If createLinearEmbed already handles the possibility where the issue URL couldn't be resolved, I wonder if we just make this, generically, \b[A-Z]{3,}-\d+\b, rather than trying to resolve issue prefixes? The advantage here is that resolving issue prefixes implies we need to periodically refresh them.


async function initializeIssueTagRegex(linearClient: LinearClient) {
const prefixes = await fetchIssuePrefixes(linearClient)
updateIssueTagRegex(prefixes)
Copy link
Contributor

Choose a reason for hiding this comment

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

Really no reason for this to be a separate function tbh.

const embedPromises = matches.map(async (match) => {
const uniqueMatches = new Set<string>()

urlMatches.forEach((match) => {
const teamName = match[1]
Copy link
Contributor

Choose a reason for hiding this comment

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

Note for all of these you can use named capturing groups.

Comment on lines +187 to +188
processedIssues.add(uniqueKey)
uniqueMatches.add(JSON.stringify({ issueId, commentId, teamName }))
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than do this, make processedIssues a Map<string, { issueId: string; commentId: string; teamName: string }>, then .set(uniqueKey, { issueId, commentId, teamName }). This collapses the processed issues and unique matches array, and then instead of Array.from(uniqueMatches) you can do Array.from(processedIssues.values()) and skip all the JSON back and forth.

Comment on lines +196 to +197
Array.from(uniqueMatches).some(
(uniqueMatch) => JSON.parse(uniqueMatch).issueId === issueId,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just uniqueMatches.has("${issueId}-")?

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.

2 participants