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

Make sure we get state/status from the right variable name when we get chat report name #4239

Merged
merged 1 commit into from
Jul 27, 2021

Conversation

yuwenmemon
Copy link
Contributor

@TomatoToaster please review

Details

I messed up here and was pulling the state/status from stateNum/statusNum rather than state/status

Fixed Issues

$ #4023

Tests/QA

See: #4023 (comment)

Made sure (deleted) was showing in the report name:
Screen Shot 2021-07-26 at 3 47 11 PM

@yuwenmemon yuwenmemon self-assigned this Jul 26, 2021
@yuwenmemon yuwenmemon requested a review from a team as a code owner July 26, 2021 22:47
@OSBotify
Copy link
Contributor

⚠️ ⚠️ Heads up! This pull request has the CP Staging label. ⚠️ ⚠️
Merging it will cause it to be immediately deployed to staging, even if the open StagingDeployCash deploy checklist is locked.

@MelvinBot MelvinBot requested review from joelbettner and removed request for a team July 26, 2021 22:47
@@ -140,8 +140,8 @@ function getChatReportName(fullReport, chatType) {
if (isDefaultRoom({chatType})) {
return `#${fullReport.reportName}${(isArchivedRoom({
chatType,
stateNum: fullReport.stateNum,
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, are we still passing stateNum from the back end, and if so is it being used anywhere in the front end?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're not. It comes from the back-end as state/status, but elsewhere in E.chat it's stateNum/statusNum

Copy link
Contributor

@TomatoToaster TomatoToaster left a comment

Choose a reason for hiding this comment

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

This LGTM! Idk how CP staging works but feel free to merge it.

@TomatoToaster TomatoToaster merged commit c00186d into main Jul 27, 2021
@TomatoToaster TomatoToaster deleted the yuwen-stateStatus branch July 27, 2021 02:41
github-actions bot pushed a commit that referenced this pull request Jul 27, 2021
@OSBotify
Copy link
Contributor

🚀 Cherry-picked to staging in version: 1.0.80-2🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Cherry-picked to production in version: 1.0.80-2🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to staging in version: 1.0.80-3🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to production in version: 1.0.81-4🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

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.

4 participants