matrix-org
Convert boolean event content to strings for push rule evaluation
Quick change so boolean event fields can be evaluated in push rules (as strings). Alternative could be a different matcher type (event_match_boolean) but that felt like overkill.
Signed of by Nick Mills-Barrett [email protected]
Pull Request Checklist
- [x] Pull request is based on the develop branch
- [x] Pull request includes a changelog file.
- [x] Pull request includes a sign off
- [x] Code style is correct (run the linters)
This does seem sensible, but I'm undecided how to handle the spec side of this.
As of https://github.com/matrix-org/matrix-doc/pull/3690 (ie, current unstable spec), the spec contains the words:
If the property specified by key ... does not have a string value, then the condition will not match.
Obviously, this PR is in conflict with that.
So we've got a couple of ways to proceed:
- Decide that this was the only sensible way to proceed, that the spec should always have allowed matching on boolean fields, that it was effectively a synapse bug that it didn't, and YOLO another PR to the spec.
- Decide that this really is a change to existing behaviour, and that we need an MSC to change the spec in this way.
Strictly speaking I think it should be an MSC, but I'm not going to insist on a bunch of process if folks think that's ridiculous.
However, it does bring into question: what should we do about other types of fields? (In particular, numbers and nulls)
Strictly speaking I think it should be an MSC, but I'm not going to insist on a bunch of process if folks think that's ridiculous.
However, it does bring into question: what should we do about other types of fields? (In particular, numbers and
nulls)
The inconsistency between non-string types is what really makes me think the behavior should be specced. It really isn't obvious to me that you would want a boolean (or number) value to be matched as a string also.
I'd rather see this go through the MSC process, I think, but would definitely defer to people with more history of how push rules are supposed to work!
While I generally do want to be able to match booleans (and other types), I'm not sure if stringifying them is the right approach.
Given the discussion - definitely feels like this should be an MSC! I've not got much time at the moment but can probably write something up soonish.
While I generally do want to be able to match booleans (and other types), I'm not sure if stringifying them is the right approach.
I'd also lean this way, string matching on booleans feels like a hack. Same for numbers/null values.
Given the discussion - definitely feels like this should be an MSC! I've not got much time at the moment but can probably write something up soonish.
For cross-linking this is MSC3758.
Closing this out, the MSC3758 takes a much better approach (match the value + type).