Page MenuHomePhabricator
Differential D9765

[keyserver] Replace undefined subthread_permissions with null
ClosedPublic
Actions

Authored by tomek on Nov 8 2023, 9:20 AM.
Tags
None
Referenced Files
F3390242: D9765.id32949.diff
Fri, Nov 29, 10:56 PM
Unknown Object (File)
Tue, Nov 26, 3:37 PM
Unknown Object (File)
Tue, Nov 26, 3:23 PM
Unknown Object (File)
Tue, Nov 26, 1:27 PM
Unknown Object (File)
Mon, Nov 25, 5:06 AM
Unknown Object (File)
Mon, Nov 25, 3:59 AM
Unknown Object (File)
Sun, Nov 24, 6:23 PM
Unknown Object (File)
Sat, Nov 2, 6:21 PM
View All 84 Files
Subscribers
will

Details

Reviewers
ashoat
atul
kamil
inka
Commits
rCOMM6949cb2ecf00: [keyserver] Replace undefined subthread_permissions with null
Summary

When fetching a message for entry action, we weren't selecting subthread_permissions. Then the result was passed to rawMessageInfoFromRows which uses JSON.parse on subthread_permissions. When a value is null, parsing returns null. But when a value is undefined, parsing throws an error.

Modified the code so that we're fetching NULL - all the entry messages are different from messageTypes.CREATE_SUB_THREAD so they don't have subthread_permissions.

https://linear.app/comm/issue/ENG-5684/editing-entries-doesnt-work

Test Plan

Create and edit an entry in the community root - it should work.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek created this revision.Nov 8 2023, 9:20 AM
Herald added a subscriber: will. · View Herald TranscriptNov 8 2023, 9:20 AM
Harbormaster returned this revision to the author for changes because remote builds failed.Nov 8 2023, 9:36 AM
Harbormaster failed remote builds in B23912: Diff 32949!
ashoat added a comment.Nov 8 2023, 9:46 AM

You should be able to rebase on master to fix the failing CommTest, but feel free to just publish now

I tried to investigate when this regression happened but I couldn't figure it out. I'm sure I was able to edit entries somewhat recently, but it appears that subthread_permissions has been in the other queries for a while, and was never in the fetchMessageInfoForEntryAction query.

keyserver/src/fetchers/message-fetchers.js
664 ↗(On Diff #32949)

Can you strip the trailing space you added to this line?

tomek updated this revision to Diff 32990.Nov 9 2023, 1:28 AM

Rebase

tomek added a comment.Nov 9 2023, 1:29 AM

I tried to investigate when this regression happened but I couldn't figure it out. I'm sure I was able to edit entries somewhat recently, but it appears that subthread_permissions has been in the other queries for a while, and was never in the fetchMessageInfoForEntryAction query.

It could be introduced in D4521. Even now, editing works in some threads, and I'm not sure why it doesn't work in some cases.

keyserver/src/fetchers/message-fetchers.js
664 ↗(On Diff #32949)

Sure!

Harbormaster returned this revision to the author for changes because remote builds failed.Nov 9 2023, 1:45 AM
Harbormaster failed remote builds in B23947: Diff 32990!
tomek requested review of this revision.Nov 9 2023, 2:01 AM
tomek updated this revision to Diff 33000.Nov 9 2023, 2:20 AM

Rebase

Harbormaster completed remote builds in B23949: Diff 33000.Nov 9 2023, 3:02 AM
inka accepted this revision.Nov 9 2023, 3:20 AM
This revision is now accepted and ready to land.Nov 9 2023, 3:20 AM
Closed by commit rCOMM6949cb2ecf00: [keyserver] Replace undefined subthread_permissions with null (authored by tomek). · Explain WhyNov 9 2023, 4:01 AM
This revision was automatically updated to reflect the committed changes.
tomek added a commit: rCOMM6949cb2ecf00: [keyserver] Replace undefined subthread_permissions with null.

Revision Contents
Changeset List

PathSize
keyserver/
src/
fetchers/
6 lines

Diff 33001

View Options

keyserver/src/fetchers/message-fetchers.js

Loading...