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

Fix Sending Stickers from Dimension doesn't work #4984

Closed
wants to merge 5 commits into from

Conversation

acheng-floyd
Copy link

Fixes: Sending Stickers from Dimension doesn't work #1497

A saclar token is needed by self deployed dimension when using sticker picker.
Fix roomid check is no needed when using sticker picker

Noted: there still problem to enable sticker picker for the first time, if we want to use sticker picker, we should enable it on element-web then we can use sticker picker in android with self deployed dimension

Here's what your changelog entry will look like:

🐛 Bug Fixes
Fixes: Sending Stickers from Dimension doesn't work #1497. Contributed by @achengfloyd

Before fix:
image

After fix:
image

Fix a saclar token is needed by self deployed dimension when using sticker picker
Fix roomid check is no needed when using sticker picker
Noted: there still problem to enable sticker picker for the first time, if we want to use sticker picker, we should enable it on element-web then we can use sticker picker in android with self deployed dimension
@@ -211,12 +212,23 @@ class WidgetPostAPIHandler @AssistedInject constructor(@Assisted private val roo
* @param eventData the modular data
*/
private fun getWidgets(widgetPostAPIMediator: WidgetPostAPIMediator, eventData: JsonDict) {
if (checkRoomId(widgetPostAPIMediator, eventData)) {
return
//if (checkRoomId(widgetPostAPIMediator, eventData)) {
Copy link
Contributor

@ouchadam ouchadam Jan 21, 2022

Choose a reason for hiding this comment

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

if it's no longer needed lets delete the code rather than commenting out ♻️

//if (checkRoomId(widgetPostAPIMediator, eventData)) {
// return
//}
val roomIdInEvent = eventData["room_id"] as String?
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if this could be simplified to

val allWidgets = when {
    eventData.isMissingRoomId() -> session.widgetService().getUserWidgets()
    roomId == eventData.roomId  -> session.widgetService().getRoomWidgets(roomId) + session.widgetService().getUserWidgets()
    else                        -> {
        widgetPostAPIMediator.sendError(stringProvider.getString(R.string.widget_integration_room_not_visible), eventData)
        return
    }
}

Copy link
Author

@acheng-floyd acheng-floyd Jan 24, 2022

Choose a reason for hiding this comment

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

That looks nice, but i try your code in my ide, it looks like this
image

I'm sorry i'm really not familiar with kotlin, i just try to use the easy way i could understand~~

Copy link
Contributor

Choose a reason for hiding this comment

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

ah my bad! this was a mix of pseudo code, these could be implemented as kotlin extension functions on the JsonDict, we could also extract another for matching against the room id

private fun getWidgets(widgetPostAPIMediator: WidgetPostAPIMediator, eventData: JsonDict) {
  val allWidgets = when {
      eventData.isMissingRoomId()        -> session.widgetService().getUserWidgets()
      eventData.hasMatchingRoom(roomId)  -> session.widgetService().getRoomWidgets(roomId) + session.widgetService().getUserWidgets()
      else                               -> {
          widgetPostAPIMediator.sendError(stringProvider.getString(R.string.widget_integration_room_not_visible), eventData)
          return
      }
  }
  ... 
}

private fun JsonDict.isMissingRoomId() = (this["room_id"] as? String).isNullOrEmpty()
private fun JsonDict.hasMatchingRoom(roomId: String) = (this["room_id"] as? String) == roomId

acheng-floyd and others added 3 commits January 24, 2022 16:22
as code view suggest, try to simplify the code
delete repeat debug message
@bmarty
Copy link
Member

bmarty commented Oct 4, 2022

OK, I have made a small mistake when fixing the conflict, the code will not compile. Can you add a sign-off to the PR, so that I can merge it to one of my branch to be able to fix the issue? Thanks @acheng-floyd

@bmarty
Copy link
Member

bmarty commented Jan 10, 2023

Stale PR, feel free to update and re-open.

@bmarty bmarty closed this Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
X-Needs-Info Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants