events: Make polls events compatible with v1 of MSC3381

[zecakeh]

Necessary for Element X to implement polls. v2 should not be necessary because the MSC is in PFCP, and it is just an update to the latest extensible events type format, so I removed it.

I decided to hide the v1 type as a serde helper instead of exposing it. That will also allow to keep recognizing unstable polls after the MSC is merged.

---

Preview Removed

[zecakeh]

I'm also pretty sure v2 was incorrect and was actually a mix between v1 and v2 😅.

[zecakeh]

Rebased on main to solve CI issue.

[zecakeh]

As it turns out, due to a recent update of the MSC, only the unstable version should be sent in rooms that don't support extensible events, and the stable version will be sent in rooms that support extensible events.

So we should actually have 2 different types, and the client should be in charge of sending the right one according to the room version.

[julioromano]

As it turns out, due to a recent update of the MSC, only the unstable version should be sent in rooms that don't support extensible events, and the stable version will be sent in rooms that support extensible events.

So we should actually have 2 different types, and the client should be in charge of sending the right one according to the room version.

Great thanks, there is not yet a room version that support the stable MSC AFAIK though, right?

[zecakeh]

Great thanks, there is not yet a room version that support the stable MSC AFAIK though, right?

Yes, that's right.

That's gonna take some work on the macros: iirc, we enforce that event types have at least a stable prefix alias, while the unstable types won't have one.

[csmith]

I don't really know enough rust/ruma/serde to actually review the changes, but I have managed to get MSC3381v1 poll start/response/end events plumbed in to the rust-matrix-sdk and out the other side to EAX Android using this. So LGTM functionality wise, thanks @zecakeh! :)

[zecakeh]

Nice, did you check if it works correctly with the current generation of Element clients?

[csmith]

Nice, did you check if it works correctly with the current generation of Element clients?

Good call. It works fine with polls sent from Element Web, but not with polls sent from legacy Element Android or legacy Element iOS:

missing field org.matrix.msc1767.text", line: 1, column: 400

The column number doesn't line up to anything useful, but I think the problem is the old mobile clients aren't sending the top-level org.matrix.msc1767.text with the fallback textual representation of the full poll (the question and options all in one: Is this a poll?\n1. Maybe\n2. No).

I'm not sure if that's permitted by the spec or not... MSC3381 v1 doesn't say it's required, but it's rather light on actual requirements. v2 does say the text fallback is required... but we're obviously not using v2 anyway. 🤔

Presumably making it an Option<String> (maybe with a default in the serde tag) would fix the parsing issue? I'll give that a try tomorrow and see if there are any other issues.

Model returned by Rust

EventTimelineItem {
    sender: "@cjs87:matrix.org",
    sender_profile: Ready(
        Profile {
            display_name: Some(
                "Chris",
            ),
            display_name_ambiguous: false,
            avatar_url: Some(
                "mxc://matrix.org/sugKcEQIwGbkxnMqYrYLJxlN",
            ),
        },
    ),
    timestamp: MilliSecondsSinceUnixEpoch(1689873757419),
    content: FailedToParseMessageLike {
        event_type: "org.matrix.msc3381.poll.start",
        error: Error("missing field `org.matrix.msc1767.text`", line: 1, column: 400),
    },
    kind: Remote(
        RemoteEventTimelineItem {
            event_id: "$W0WbEwSRdDzCywE1WgJoRuVZSKWQ6jVSf0ftjQmNGQI",
            reactions: {},
            read_receipts: {
                "@cjs87:matrix.org": Receipt {
                    ts: Some(
                        MilliSecondsSinceUnixEpoch(1689873757419),
                    ),
                    thread: Unthreaded,
                },
            },
            is_own: false,
            is_highlighted: false,
            encryption_info: Some(
                // 
            ),
            origin: Sync,
            ..
        },
    ),
}

Raw JSON

{"content":{"org.matrix.msc3381.poll.start":{"answers":[{"id":"0246C17E-6AC9-4003-86DF-78BEEE9DAE32","org.matrix.msc1767.text":"Op 1"},{"id":"630E95BE-19CC-4CDC-A5DE-0BFE6435D59F","org.matrix.msc1767.text":"Op 2"},{"id":"208BFA78-430A-4188-8982-791BEED270C2","org.matrix.msc1767.text":"Op 3"}],"kind":"org.matrix.msc3381.poll.disclosed","max_selections":1,"question":{"org.matrix.msc1767.text":"Poll from EI"}}},"event_id":"$W0WbEwSRdDzCywE1WgJoRuVZSKWQ6jVSf0ftjQmNGQI","origin_server_ts":1689873757419,"room_id":"!pQeRyVUiziTXUMgRdx:matrix.org","sender":"@cjs87:matrix.org","type":"org.matrix.msc3381.poll.start","unsigned":{"age":206}}

Formatted JSON

{
    "content":
    {
        "org.matrix.msc3381.poll.start":
        {
            "answers":
            [
                {
                    "id": "0246C17E-6AC9-4003-86DF-78BEEE9DAE32",
                    "org.matrix.msc1767.text": "Op 1"
                },
                {
                    "id": "630E95BE-19CC-4CDC-A5DE-0BFE6435D59F",
                    "org.matrix.msc1767.text": "Op 2"
                },
                {
                    "id": "208BFA78-430A-4188-8982-791BEED270C2",
                    "org.matrix.msc1767.text": "Op 3"
                }
            ],
            "kind": "org.matrix.msc3381.poll.disclosed",
            "max_selections": 1,
            "question":
            {
                "org.matrix.msc1767.text": "Poll from EI"
            }
        }
    },
    "event_id": "$W0WbEwSRdDzCywE1WgJoRuVZSKWQ6jVSf0ftjQmNGQI",
    "origin_server_ts": 1689873757419,
    "room_id": "!pQeRyVUiziTXUMgRdx:matrix.org",
    "sender": "@cjs87:matrix.org",
    "type": "org.matrix.msc3381.poll.start",
    "unsigned":
    {
        "age": 206
    }
}

Ele-web JSON for comparison

{
    "content":
    {
        "org.matrix.msc1767.text": "Is this a poll?\n1. Maybe\n2. No",
        "org.matrix.msc3381.poll.start":
        {
            "answers":
            [
                {
                    "id": "Ed8y02SXw62sxUwQ",
                    "org.matrix.msc1767.text": "Maybe"
                },
                {
                    "id": "8d39SVk2XFQdHKiH",
                    "org.matrix.msc1767.text": "No"
                }
            ],
            "kind": "org.matrix.msc3381.poll.disclosed",
            "max_selections": 1,
            "question":
            {
                "body": "Is this a poll?",
                "msgtype": "m.text",
                "org.matrix.msc1767.text": "Is this a poll?"
            }
        }
    },
    "event_id": "$B4pFF80CbDOE6dPMRGlnG3MvvMzgfFF9sknXn3Z4aZc",
    "origin_server_ts": 1689763547640,
    "room_id": "!pQeRyVUiziTXUMgRdx:matrix.org",
    "sender": "@cjs87:matrix.org",
    "type": "org.matrix.msc3381.poll.start",
    "unsigned":
    {
        "age": 308
    }
}

[csmith]

I'm not sure if that's permitted by the spec or not... MSC3381 v1 doesn't say it's required, but it's rather light on actual requirements. v2 does say the text fallback is required... but we're obviously not using v2 anyway. thinking

Just as I was closing the tab I noticed this line in MSC3881 v1:

The m.message fallback should be representative of the poll, but is not required and has no mandatory format. Clients are encouraged to be inspired by the example above when sending poll events.

So yeah, not required by the spec, and apparently EX and EI weren't particularly "inspired" 😆

[zecakeh]

Alright, that's gonna be an easy change to make. Have you checked if it's the same behavior in the poll end events?

[csmith]

Have you checked if it's the same behavior in the poll end events?

I'd checked ending in web and Element iOS (which both work fine), but I just tried Android as well to be thorough and... it doesn't work. Seems like it's omitting the empty org.matrix.msc3381.poll.end object:

    content: FailedToParseMessageLike {
        event_type: "org.matrix.msc3381.poll.end",
        error: Error("missing field `org.matrix.msc3381.poll.end`", line: 1, column: 150),
    },

JSON

{
    "content":
    {
        "body": "",
        "m.relates_to":
        {
            "event_id": "$FBv6Xxlsgmg98t7Hve7jUK7EBgfSxZn0CdHhZMpwu7Q",
            "rel_type": "m.reference"
        },
        "org.matrix.msc1767.text": "Ended poll"
    },
    "event_id": "$diODf-uVhyK5wHS9f1-3QC8d_n2n4k2Fw7k8sgitkAE",
    "origin_server_ts": 1689932821757,
    "room_id": "!pQeRyVUiziTXUMgRdx:matrix.org",
    "sender": "@cjs87-test:matrix.org",
    "type": "org.matrix.msc3381.poll.end",
    "unsigned":
    {
        "age": 89
    }
}

I think that's actually a spec violation from EA, but EA/EI/EWeb all happily parse it

[zecakeh]

Alright, there should be no more issues with this last commit

[csmith]

Confirmed it now parses {start, response end} events from Element {Android, iOS, Web}. Thanks! 🎉

[jonnyandrew]

Not a complete review, just a few things that jumped out at me