Skip to content

COD-1331 Add action item to open the context on vscode notifications#451

Merged
meichend merged 3 commits into
developmentfrom
meichendong/cod-1331-might-be-a-good-idea-to-add-an-action
Jul 19, 2022
Merged

COD-1331 Add action item to open the context on vscode notifications#451
meichend merged 3 commits into
developmentfrom
meichendong/cod-1331-might-be-a-good-idea-to-add-an-action

Conversation

@meichend
Copy link
Copy Markdown
Contributor

Description

When the notification is bout a context, make the action button go to the context.

Type of Change

  • Tech Debt (refactoring, unit tests, CI changes, pre-commit hooks, etc)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

🤲

Test Configuration:

  • CCL version/branch: 1.X
  • API version/branch: 1.X

Checklist:

  • I have performed a self-review of my code
  • I have used codex to provide context, particularly in hard-to-understand areas
  • Any dependent changes have been made available in downstream modules

Screenshots:

image

@meichend meichend requested a review from cpul July 18, 2022 19:21
@linear
Copy link
Copy Markdown

linear Bot commented Jul 18, 2022

COD-1331 Might be a good idea to add an action item to open the context here

Example: https://stackoverflow.com/questions/57416105/how-to-make-a-click-event-in-vscode-window-showinformationmessage

image.png

Only handle context's & comments in this story. (Send to context details page)

Change copy to 'Open' in those instances

@meichend
Copy link
Copy Markdown
Contributor Author

image

uhhh why am I receiving this on my end

if (selection == OPEN) {
await vscode.commands.executeCommand("workbench.view.extension.codex-sidebar-view");
const codexService = new CodexService(notification.codexNotificationObject.codexId, UserService.getInstance());
const context: Context = ((await codexService.getContextService().getOneById(contextId)) as Context[])[0];
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.

What happens if there's no context? You may get an error here trying to access something that may not exist.

Copy link
Copy Markdown
Contributor

@cpul cpul left a comment

Choose a reason for hiding this comment

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

I'm good with this change and would leaving an approval so I don't block you.
I would like to see a quick update though where you guard against an index out of bounds error on the array access you're making that I left a comment on. Thanks!

@meichend meichend merged commit b3eae1c into development Jul 19, 2022
@meichend meichend deleted the meichendong/cod-1331-might-be-a-good-idea-to-add-an-action branch July 19, 2022 20:57
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