Page MenuHomePhabricator
Differential D13852

[keyserver] new fetchPrivilegedThreadInfos function
ClosedPublic
Actions

Authored by varun on Thu, Oct 31, 10:42 PM.
Tags
None
Referenced Files
F3332920: D13852.id45568.diff
Thu, Nov 21, 2:32 AM
F3332678: D13852.diff
Thu, Nov 21, 1:09 AM
Unknown Object (File)
Mon, Nov 18, 11:24 AM
Unknown Object (File)
Sun, Nov 17, 4:40 PM
Unknown Object (File)
Sun, Nov 17, 4:40 PM
Unknown Object (File)
Sun, Nov 17, 4:40 PM
Unknown Object (File)
Sun, Nov 17, 4:40 PM
Unknown Object (File)
Sun, Nov 17, 11:44 AM
View All 26 Files
Subscribers
tomek

Details

Reviewers
ashoat
Commits
rCOMM1490b842faf6: [keyserver] new fetchPrivilegedThreadInfos function
Summary

unlike fetchThreadInfos, this function does not filter out threads that are inaccessible to the given user

Depends on D13851

we need this function to get all the threadInfos for community root threads. we'll use this data on clients to populate a directory of communities for users to join

Test Plan

successfully got back threadInfos for all threads when calling responder in next diff

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

varun created this revision.Thu, Oct 31, 10:42 PM
Herald added a subscriber: tomek. · View Herald TranscriptThu, Oct 31, 10:42 PM
varun added a child revision: D13853: [keyserver] add know_of permission if viewer.isScriptViewer.Thu, Oct 31, 10:46 PM
Harbormaster completed remote builds in B32504: Diff 45568.Thu, Oct 31, 10:58 PM
varun requested review of this revision.Thu, Oct 31, 10:58 PM
ashoat requested changes to this revision.Fri, Nov 1, 8:32 AM

This won't work without addressing the KNOW_OF issue

This revision now requires changes to proceed.Fri, Nov 1, 8:32 AM
varun updated this revision to Diff 45841.Fri, Nov 15, 10:17 AM
varun retitled this revision from [keyserver] new fetchThreadInfos function to [keyserver] new fetchPrivilegedThreadInfos function.
varun edited the summary of this revision. (Show Details)
varun edited the test plan for this revision. (Show Details)

new approach without using script viewer

varun updated this revision to Diff 45843.Fri, Nov 15, 10:19 AM

make param read only

Harbormaster failed remote builds in B32681: Diff 45843!Fri, Nov 15, 10:25 AM
ashoat requested changes to this revision.Fri, Nov 15, 10:31 AM

Don't love that we're artifically inserting a permission that doesn't exist. Is it truly necessary?

lib/shared/thread-utils.js
764 ↗(On Diff #45843)

If my comment below makes sense, I think we should rename this to something like dontFilterMissingKnowOf, to make it clear that it affects the filtering behavior

887–889 ↗(On Diff #45843)

Instead of artificially adding a KNOW_OF permission that doesn't exist, can't we just skip this check here?

This revision now requires changes to proceed.Fri, Nov 15, 10:31 AM
Harbormaster completed remote builds in B32679: Diff 45841.Fri, Nov 15, 10:37 AM
varun updated this revision to Diff 45891.Tue, Nov 19, 9:24 PM

address feedback

varun added inline comments.Tue, Nov 19, 9:25 PM
lib/shared/thread-utils.js
878 ↗(On Diff #45891)

confirmed that this returned the same results as adding the KNOW_OF permission (previous approach)

Harbormaster completed remote builds in B32719: Diff 45891.Tue, Nov 19, 9:41 PM
ashoat accepted this revision.Wed, Nov 20, 4:13 AM
This revision is now accepted and ready to land.Wed, Nov 20, 4:13 AM
Closed by commit rCOMM1490b842faf6: [keyserver] new fetchPrivilegedThreadInfos function (authored by varun). · Explain WhyWed, Nov 20, 5:20 AM
This revision was automatically updated to reflect the committed changes.
varun added a commit: rCOMM1490b842faf6: [keyserver] new fetchPrivilegedThreadInfos function.

Revision Contents
Changeset List

PathSize
keyserver/
src/
fetchers/
14 lines
lib/
shared/
7 lines

Diff 45896

View Options

keyserver/src/fetchers/thread-fetchers.js

Loading...
View Options

lib/shared/thread-utils.js

Loading...