-
Notifications
You must be signed in to change notification settings - Fork 1
Add Supabase integration for PHLask resource management #17
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
base: main
Are you sure you want to change the base?
Conversation
Implements utility functions and type definitions for interacting with the PHLask resource database, enabling the admin dashboard to fetch, edit, and delete resource entries with support for pagination and filtering. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
RNR1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass! Thanks for adding these here
| } | ||
|
|
||
| export default function Home() { | ||
| useEffect(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In React Router framework, we should use loaders instead of useEffect, then access the data in the component using the useLoaderData hook.
https://reactrouter.com/start/framework/data-loading
| // Fetch and log resources when the page loads | ||
| const fetchAndLogResources = async () => { | ||
| try { | ||
| console.log("🔄 Fetching PHLask resources..."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should remove the logs from the PR before merging
| */ | ||
| export interface DataSource { | ||
| /** The type of data source */ | ||
| type: 'MANUAL' | 'WEB_SCRAPE'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for union types: can we declare them separately? I can see cases where that we might want to type certain data separately if we submit it within the app. That will also be cleaner for arrays of union types
| /** Whether or not this resource is currently verified */ | ||
| verified: boolean; | ||
| /** The latest date this resource had a verification change */ | ||
| last_modified: Date; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for dates - when we submit data, or receive it from the response, is it already parsed to a date object? We might have to use a Date | string otherwise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, I converted this file to typescript in my current open PR on the map repo
| } | ||
|
|
||
| return { | ||
| data: (data || []) as ResourceEntry[], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When looking at the Supabase docs, I saw that they provide a CLI tool for generating types from the data. I didn't have db access so I couldn't try it myself, but if you can make the database instance type aware that would be the most ideal. Ideally, we don't type cast and use generics when needed
| .range(offset, offset + limit - 1); | ||
|
|
||
| // Apply filters if provided | ||
| if (resourceType) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the filtering here!
| .single(); | ||
|
|
||
| if (error) { | ||
| if (error.code === 'PGRST116') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does supabase provides enums for these error codes? Can we have a more descriptive and human-readable mapping of these errors?
| * @param lon2 - Longitude of the second point | ||
| * @returns Distance in meters | ||
| */ | ||
| function calculateDistance( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: move this to its own util file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Since we have multiple models, can we have a models or api directory with a file to represent each one of them? We should also think about a more unified structure for each model, something like
// types/api.ts
type ModelAPI<Entity, Params> = {
getList: (params: Params) => Promise<Entity[]>,
getById: (id: string) => Promise<Entity>,
create: (data: Entity) => Promise<Entity>,
updateById: (id: string, data: Partial<Entity>) => Promise<Entity>,
delete: (id: string) => Promise<void>,
}
// api/ResourceEntry.ts
const ResourceEntryAPI: ModelAPI<ResourceEntry, ResourceEntryGetListParams> = {
// implementation
}- We should have general DB utils. I’d suggest avoiding using the technology name (Supabase) for the file name, since technologies change often, and we don’t want that to be a factor if we decide to switch. Let’s call it something more generic, like db.ts, similar to how we have done at the map app
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing this to request changes because we don’t have any merge protection on this repo yet
Implements utility functions and type definitions for interacting with the PHLask resource database, enabling the admin dashboard to fetch, edit, and delete resource entries with support for pagination and filtering. For now, I have the home page print this to the console so others can see this in action.
Addresses #16
Note that this include the pnpm.lock file since lock files should be commited with projects. Also note this includes the API keys exactly like they are committed in the project for phlask-map (we still need to do domain locking at some point)
🤖 Generated with Claude Code