Added new MessageSpec for editing messages. Registered it in the MessageSpecs.
Details
Details
Checked if code compiles. Run app on the web and native. Then added new message with new type to database, and checked if the app still works, and loads properly the new message type by logging info to the console. Couldn't check yet all scenarios (like creating new message type locally on client side)
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
I think we should consider if we also need shimUnsupportedMessageInfo and unshimMessageInfo. These functions will help us support older native clients that haven't been updated yet.
| lib/shared/messages/edit-message-spec.js | ||
|---|---|---|
| 39 ↗ | (On Diff #23476) | |
| 55 ↗ | (On Diff #23476) | Should this be row.content.text? Also in reaction-message-spec I parsed row.content into JSON before returning it? should we do the same here? |
| 110–112 ↗ | (On Diff #23476) | This can be simplified to generatesNotifs: async () => undefined, |
Comment Actions
Yes. You are right. I will add it in another diff.
| lib/shared/messages/edit-message-spec.js | ||
|---|---|---|
| 55 ↗ | (On Diff #23476) | I store the new version of the message right in the content column in the database. For this reason, parsing it into JSON isn't necessary. Here is a screenshot from the DB:  |
Comment Actions
Is it possible we'll want to add some metadata to the content field later? I think it would be safer to use JSON
Comment Actions
I don't know how to test message shimming yet, so for now it isn't tested.
| lib/shared/messages/edit-message-spec.js | ||
|---|---|---|
| 121–123 | I wasn't sure what version should I add here. | |
Comment Actions
The proper way of testing older clients involves running an older client in prod mode (so that it won't get hot reloaded when we update the code version), then checking out the newest code, and running the server with this code. In this case we can consider a simplified approach: the thing which interests us the most is if the server sends an original message or the shimmed version. To test that we can run a client and set code version in the condition on the keyserver. If the client's version is higher than server's, we should receive an original message. Otherwise, it should be the shimmed one.
| lib/shared/messages/edit-message-spec.js | ||
|---|---|---|
| 121–123 | We usually put some high code version, because we're not sure in which version the support for new message type will be included. E.g. 1000 might work ok. | |
Comment Actions
Thanks for the help! Here is how it looks after changing the version to a higher one:
However, I'm not sure if this is what the user would like to see, because in reality that robotexts don't contain any information, but they can take up a lot of space.
Comment Actions
Nice thanks for addressing my comments and adding shimUnsupportedMessageInfo and unshimMessageInfo . Just make sure before you land that you address @ashoat's comment above and please make note somewhere or create a linear task to set the correct code version when we launch this feature
| lib/shared/messages/edit-message-spec.js | ||
|---|---|---|
| 121–123 ↗ | (On Diff #23980) | Change minCodeVersion before landing |