-
Notifications
You must be signed in to change notification settings - Fork 2
feat: Enhance template with Dog & Owner tables, relationships, and custom logic #14
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
| tag: String @indexed # we can specify any attributes that should be indexed | ||
| ## This example uses Owners and Dogs, with a relationship and a computed value. | ||
|
|
||
| type Owner @table(database: "application") @export { |
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.
Do we recommend splitting out from data? I have a vague recollection of sticking with data being the recommendation, unless someone is starting to really branch out into multiple apps 🤷 I don't have strong feelings on the topic.
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.
Yes, we recommend using data for the application; one less thing to think about (unless, like Dawson said, there are multiple applications being independently).
| dogIds: [ID] @indexed | ||
|
|
||
| # Computed: how many dogs this owner has, based on dogIds. | ||
| owner_dog_count: Int @computed(version: 1) |
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.
⛏️ Thoughts on changing it to ownerDogCount: Int @computed(version: 1), and any references? For consistency.
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.
Oh yeah good call i didn't even realize I was using both snake and camel haha
| dogs: [Dog] @relationship(from: dogIds) | ||
| } | ||
|
|
||
| type Dog @table(database: "application") @export { |
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.
Thoughts on @sealeding this too?
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.
yes good call
|
|
||
| // Example: custom resource using relationships | ||
| // | ||
| // This resource is not tied to a specific table. It demonstrates how to: |
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.
I'm guessing if you spell out too that you can GET /OwnerHasBreed to hit this, and that it will be case-sensitive, right? That should help avoid confusion.
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.
I did notice you highlighting it in the README, but adding a reference here too, even as simple as GET /OwnerHasBreed, should cut down on confusion.
|
This seems like it is an example, not a template. Why don't we move this example to its own repository? (It looks like a fantastic example). |
dawsontoth
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.
I like it!
|
That's a good point, Kris. There may be enhancements from this that could belong in the template, though. Slightly better explanations. But not verging into an example... |
| }; | ||
|
|
||
| let owner = null; | ||
| for await (const o of OwnerTable.search(query)) { |
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.
Why not just do OwnerTable.get(ownerName) instead of this whole query?
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.
Yeah I probably should switch it. I wanted to have some example in here where we are showing how to query, especially with nested relationships
| // Validate all required fields | ||
| const missingFields = []; | ||
| if (!record.id) missingFields.push('id'); | ||
| if (!record.name) missingFields.push('name'); |
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.
Do we want to be encouraging users to roll their own validation rather than using built-in validation?
|
This reminds me of the changes I made to the Next.js Example a little bit ago. There is a fine line between a "template" and an "example". I think this may still be simple enough to count as a template. I agree with the feedback from Dawson and Kris. I think this could be simplified just a bit to remain as a template. |
|
I think our vision is to have a guided schema creation tool that helps users create exactly the table structure they want. If creating a new application involves creating a table and deleting example tables, that's a lot of extra and unnecessary friction, I think. I believe there is definitely value in example applications, but the purpose of the application-template is really to be a clean slate from which we can help users build the application they want. |
In that regard, should we remove the content from all files in this template? Should things like |
This PR enhances the application template by introducing Dog and Owner tables, including computed fields, relationships, and custom resource logic. It demonstrates slightly more advanced Harper features and provides a more comprehensive foundation for building real-world applications.