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.

Sign up for GitHub

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

Jump to bottom

Jim/FEQ-300/sendbird-connection-should-be-made-first #9378

Merged
merged 23 commits into from
Jul 31, 2023
Merged

Jim/FEQ-300/sendbird-connection-should-be-made-first #9378

merged 23 commits into from
Jul 31, 2023

Conversation

jim-deriv
Copy link
Contributor

@jim-deriv jim-deriv commented Jul 17, 2023 •
edited
Loading

Changes:

  • Upgraded sendbird from v3 (which is deprecated) to v4 and added checks to prevent making calls to the sendbird api, before the channel_url is set.
  • Migrated the sendbird_store and chat-message to TypeScript

Migration guide: sendbird

@boring-cyborg boring-cyborg bot added the P2P label Jul 17, 2023
@vercel
Copy link

vercel bot commented Jul 17, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
deriv-app ✅ Ready (Inspect) Visit Preview Jul 27, 2023 3:45am

@github-actions
Copy link
Contributor

github-actions bot commented Jul 17, 2023 •
edited
Loading

A production App ID was automatically generated for this PR. (log)

Click here to copy & paste above information. 
- **PR**: [https://github.com/binary-com/deriv-app/pull/9378](https://github.com/binary-com/deriv-app/pull/9378)
- **URLs**:
    - **w/ App ID + Server**: https://deriv-app-git-fork-jim-deriv-jim-feq-300sendbird-connect-1bd370.binary.sx?qa_server=red.binaryws.com&app_id=24075
    - **Original**: https://deriv-app-git-fork-jim-deriv-jim-feq-300sendbird-connect-1bd370.binary.sx
- **App ID**: `24075`

@github-actions
Copy link
Contributor

github-actions bot commented Jul 17, 2023 •
edited
Loading

🚨 Lighthouse report for the changes in this PR:

Category Score
🔺 Performance 21
🟧 Accessibility 75
🟢 Best practices 92
🟧 SEO 85
🟢 PWA 90

Lighthouse ran with https://deriv-app-git-fork-jim-deriv-jim-feq-300sendbird-connect-1bd370.binary.sx/

jim-deriv
jim-deriv commented Jul 18, 2023
View reviewed changes
packages/p2p/src/stores/sendbird-store.ts
}

sendMessage(message) {
const modified_message = message.replace(/^[\r\n]+|[\r\n]+$/g, '');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not quite sure why the original author didn't use message.trim() here, because it seems like the regex just matches and replace new line or return carriage characters. Will add it to QA checklist to verify this, because the regex causes a security hotspot, so I decided to replace it with .trim().

@coveralls
Copy link

coveralls commented Jul 18, 2023 •
edited
Loading

Coverage Status

coverage: 8.87% (-0.001%) from 8.871% when pulling e8138ea on jim-deriv:Jim/FEQ-300/sendbird-connection-should-be-made-first into ca31c81 on binary-com:master.

@jim-deriv jim-deriv marked this pull request as ready for review July 18, 2023 03:59
@github-actions
Copy link
Contributor

⏳ Generating Lighthouse report...

@sonarcloud
Copy link

sonarcloud bot commented Jul 27, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@amina-deriv amina-deriv merged commit 64ac3ae into deriv-com:master Jul 31, 2023
3 checks passed
matin-deriv pushed a commit to matin-deriv/deriv-app that referenced this pull request Aug 2, 2023
@jim-deriv @matin-deriv
Jim/FEQ-300/sendbird-connection-should-be-made-first (deriv-com#9378)
c930cc0 
* chore: upgrade sendbird to v4

* chore: await async method calls

* chore: await aync method calls

* chore: resolve code smells

* chore: resolve code smells

* fix: fix failing tests

* chore: add types and await async method calls
@farhan-nurzi-deriv farhan-nurzi-deriv mentioned this pull request Aug 4, 2023
Merged
@jim-deriv jim-deriv deleted the Jim/FEQ-300/sendbird-connection-should-be-made-first branch November 23, 2023 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@farhan-nurzi-deriv farhan-nurzi-deriv farhan-nurzi-deriv approved these changes

@ameerul-deriv ameerul-deriv ameerul-deriv approved these changes

@yauheni-deriv yauheni-deriv yauheni-deriv approved these changes

@maryia-deriv maryia-deriv maryia-deriv approved these changes

@shayan-deriv shayan-deriv shayan-deriv approved these changes

Assignees
No one assigned
Labels
P2P
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@jim-deriv @coveralls @shayan-deriv @maryia-deriv @yauheni-deriv @ameerul-deriv @farhan-nurzi-deriv @amina-deriv