Page MenuHomePhabricator

[lib] change fetchFarcasterChannelByName -> fetchFarcasterChannelByID and refactor
ClosedPublic

Authored by varun on Oct 15 2024, 12:44 PM.
Tags
None
Referenced Files
F3311512: D13718.id45247.diff
Tue, Nov 19, 9:23 AM
Unknown Object (File)
Sat, Nov 16, 10:48 AM
Unknown Object (File)
Sat, Nov 16, 10:47 AM
Unknown Object (File)
Sat, Nov 16, 10:47 AM
Unknown Object (File)
Fri, Nov 15, 4:59 AM
Unknown Object (File)
Fri, Nov 15, 2:46 AM
Unknown Object (File)
Thu, Nov 14, 3:56 PM
Unknown Object (File)
Thu, Nov 14, 3:51 PM
Subscribers

Details

Summary

realized we're conflating channel name and ID in our code. this change cleans things up a little and makes sure we're querying by ID

Depends on D13695

Test Plan

called fetchFarcasterChannelByID with a channel name ("Crypto Fantasy League") and didn't get back a response. called it again with a channel ID ("fantasyisland") and got the right response

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

varun published this revision for review.Oct 15 2024, 12:46 PM

I see D13717 was landed already but I think this diff is still an improvement. Will rebase and wait for CI before landing

lib/utils/neynar-client.js
152 ↗(On Diff #45207)

Even though we only call this function from fetchFarcasterChannelByID, I think it's a useful change. we may want to query by name in the future and both types of queries use the same underlying neynar api

ashoat requested changes to this revision.Oct 15 2024, 2:00 PM

Glad you independently found the id/name bug!

I don't think we need support for fetching by name right now. Can you simplify this to just a rename?

lib/utils/neynar-client.js
152 ↗(On Diff #45207)

I think this is overengineering. I don't think it's worth having this complexity in the codebase for a hypothetical future use case. We can always introduce it later when we determine that use case is needed. In general, this approach of building for hypothetical cases hurts readability and bloats the codebase, increasing the amount of maintenance required

This revision now requires changes to proceed.Oct 15 2024, 2:00 PM
This revision is now accepted and ready to land.Oct 16 2024, 9:13 PM