Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lexicons: add more search parameters #2396

Merged
merged 11 commits into from
Apr 11, 2024
Merged

lexicons: add more search parameters #2396

merged 11 commits into from
Apr 11, 2024

Conversation

bnewbold
Copy link
Collaborator

@bnewbold bnewbold commented Apr 9, 2024

Let's get agreement on the params/lexicon structure, then might need to hand this off to dan/devin to implement the query param pass-through in the appview typescript code (or I can try to bite that off).

Lexicon style notes:

  • using full tokens/refs for things like sort-order felt a bit heavy? on the other hand would feel more lexicon-idiomatic
  • I didn't specify default even for fields which have a clear default. I don't think we've used default much? and i'm not sure if that would result in the query-param always getting set by client, or only interpreted on backend. I guess writing this now I would lean towards making the defaults explicit
  • in theory, more of these params could be arrays instead of single string values. that would help things like bots and analysis tools, but could make queries expensive, and API a bit less ergonomic in languages like golang. would always be easy to add plural version ("langs") in the future? hrm.

There are a couplbut I guess particularly with query params it would make sense.
e additional post search params we might want to add, but might require more palomar work so haven't added yet:

  • adult (boolean?): whether to filter out posts with adult-content images. kind of redundant with labeler header, but can be more efficient to do this in the search index itself
  • replies (boolean?): whether to include reply posts, or only top-level posts
  • media (string?): eg, no-images, only-images, only-video-embed

@@ -16,6 +16,11 @@
"type": "string",
"description": "Search query string. Syntax, phrase, boolean, and faceting is unspecified, but Lucene query syntax is recommended."
},
"account": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to call this actor or for or something? Not sure if we've formalized a name for this elsewhere but I don't recall using account in other places for this, I might just not be super immersed in lexicons though.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only place we use "account" so far is for a user on a pds, who may not necessarily have a repository on the network. I think actor fits but would be ambiguous. My only issue with for is that it's a keyword in many languages 😆 Another option that comes to mind is viewer, which is the name of the field where we put viewer-specific data for many response lexicons. If we land on "account" I wont be sad, though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like viewer, or sticking with account as a second option.

"format": "at-identifier",
"description": "Filter to posts by the given account. Handles are resolved to DID before query-time."
},
"lang": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to support multiple values for this one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for from I assume, with "OR" semantics?

I thought about it and leaned against, though not a strong opinion. I'm guessing that folks might want to try to hack in "list search" functionality using this. But these are query params, so only would work with a couple dozen accounts (?). I think the better functionality would be actual list search as a follow-up thing, where we fetch list membership server-side.

@bnewbold bnewbold force-pushed the bnewbold/search-params branch from ee6d27b to 57cd4ff Compare April 11, 2024 23:41
@bnewbold bnewbold merged commit a2d5658 into main Apr 11, 2024
10 checks passed
@bnewbold bnewbold deleted the bnewbold/search-params branch April 11, 2024 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants