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

VIDCS-3205: Fix deps security issues with resolutions #7

Merged
merged 2 commits into from
Jan 15, 2025

Conversation

maikthomas
Copy link
Contributor

@maikthomas maikthomas commented Jan 14, 2025

What is this PR doing?

How should this be manually tested?

  • Check that mend CI check passes
  • Check tests pass

What are the relevant tickets?

Resolves VIDCS-3205

[ ] Resolves a Known Issue.
[ ] If yes, did you remove the item from the docs/KNOWN_ISSUES.md?
[ ] Resolves an item reported in Issues.
If yes, which issue? Issue Number?
[ ] If yes, did you close the item in Issues?

Comment on lines +74 to +81
"wrap-ansi": "7.0.0",
"path-to-regexp": "^0.1.12",
"express": "^4.21.2",
"axios": "^1.7.4",
"cross-spawn": "^7.0.5",
"rollup": "^4.22.4",
"vite": "^5.4.11",
"nanoid": "^3.3.8"
Copy link
Contributor Author

@maikthomas maikthomas Jan 14, 2025

Choose a reason for hiding this comment

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

Unfortunately we have dependencies that do not have a compatible version which updates these internal dependencies so we have to use yarn resolutions to override those versions instead.

@maikthomas maikthomas changed the title VIDCS-3205 Fix deps security issues with resolutions VIDCS-3205: Fix deps security issues with resolutions Jan 14, 2025
@maikthomas maikthomas closed this Jan 14, 2025
@maikthomas maikthomas reopened this Jan 14, 2025
Comment on lines 135 to 137
await pageOne.waitForSelector('.publisher', { state: 'visible' });
await pageTwo.waitForSelector('.subscriber', { state: 'visible' });
const screenshareButton = await pageOne.getByTestId('ScreenShareIcon');
Copy link
Contributor Author

@maikthomas maikthomas Jan 14, 2025

Choose a reason for hiding this comment

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

I think this was failing sometimes because we weren't connected so the screenshare button wasn't available yet.
For now this fix waits until a subscriber is visible for tab 2 which means we're obviously already connected and publishing & subscribing - then starts screensharing from page one

Copy link
Contributor

@behei-vonage behei-vonage left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

Copy link
Collaborator

@v-kpheng v-kpheng left a comment

Choose a reason for hiding this comment

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

LGTM! 💪 🚀

@behei-vonage
Copy link
Contributor

since the relevant CI checks pass I will merge this PR in

@behei-vonage behei-vonage merged commit 0b3fe3a into main Jan 15, 2025
9 of 10 checks passed
@behei-vonage behei-vonage deleted the mthomas/VIDCS-3205-fix-deps-security branch January 15, 2025 21:50
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.

3 participants