Skip to content

Domain Models#168

Merged
TheJolman merged 32 commits intomainfrom
feat/Domain-Models
Apr 22, 2026
Merged

Domain Models#168
TheJolman merged 32 commits intomainfrom
feat/Domain-Models

Conversation

@GaballaGit
Copy link
Copy Markdown
Member

This PR adds Domain Models to the API.

@GaballaGit GaballaGit requested a review from sidvasu as a code owner March 1, 2026 23:34
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 1, 2026

godoc reference preview ready ✨
Go Documentation

@GaballaGit
Copy link
Copy Markdown
Member Author

NOTE: THE MAPPER ISN'T SAFE TO MERGE YET. There are some pointer dereferences here that can cause a panic, as well as the time.Time pointers returning misleading values if the pointer is nil.

@GaballaGit GaballaGit marked this pull request as draft March 10, 2026 20:30
@GaballaGit GaballaGit marked this pull request as ready for review March 23, 2026 19:04
@GaballaGit GaballaGit requested a review from TheJolman as a code owner March 23, 2026 19:04
@GaballaGit
Copy link
Copy Markdown
Member Author

@TheJolman after 500 gajillion years I think this pr is ready for review, sorry it took me long

Copy link
Copy Markdown
Collaborator

@TheJolman TheJolman left a comment

Choose a reason for hiding this comment

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

The whole point of the domain layer is to not depend on database types in the handler layer. This means that the handler package should not be importing dbmodels and the service package should take the domain models as input.

@GaballaGit
Copy link
Copy Markdown
Member Author

The whole point of the domain layer is to not depend on database types in the handler layer. This means that the handler package should not be importing dbmodels and the service package should take the domain models as input.

Services must take a dbmodel to do db operations, there is no layer to hand over a domain as of now. Repository layer would need to be implemented so service can use domain and we can finally swap out dbmodels in handler.

@GaballaGit
Copy link
Copy Markdown
Member Author

Actually, I could probably just swap domain to db model in the service for now. Is that what you are looking for?

@TheJolman
Copy link
Copy Markdown
Collaborator

@GaballaGit yeah that's fine. And IMO implementing the repository layer isn't super important atm so don't worry about it at least until all the dtos and domain models are implemented. Ty for ur work as always 🫡

@GaballaGit
Copy link
Copy Markdown
Member Author

only thing left with this pr should be to fix the shitty naming convention in mapper

@GaballaGit
Copy link
Copy Markdown
Member Author

@TheJolman hihi, what do you think now?

@TheJolman
Copy link
Copy Markdown
Collaborator

@TheJolman hihi, what do you think now?

MB just seeing this now, but this look pretty good. tbh I'm not a huge fan of the monolithic mapper package so if you don't mind I'm gonna try to iterate on the mapping logic and see if I can get something that feels better (I'm probably going to used files like announcement_mapper.go or something like that).

@GaballaGit
Copy link
Copy Markdown
Member Author

wtf

@GaballaGit
Copy link
Copy Markdown
Member Author

Intresting behavior to note right now:
Getting by id displays the announceAt time in unix time, while getting all announcements displays announceAt in ISO 8601 format.

image

@GaballaGit
Copy link
Copy Markdown
Member Author

@TheJolman @sidvasu I need some thoughts here, the cli now returns the dto model when we use it. Therefor we receive a Unix format in any time slot which reduces the human friendliness of the cli output. Should we figure out a way around this? Or should we keep it like this.

@TheJolman
Copy link
Copy Markdown
Collaborator

@TheJolman @sidvasu I need some thoughts here, the cli now returns the dto model when we use it. Therefor we receive a Unix format in any time slot which reduces the human friendliness of the cli output. Should we figure out a way around this? Or should we keep it like this.

Currently the CLI just outputs the raw JSON response, correct? I think this behavior is fine actually, and we can make the CLI translate it to something human readable in the future.

@GaballaGit
Copy link
Copy Markdown
Member Author

Sounds good, I think this one is ready for a merge!

@TheJolman
Copy link
Copy Markdown
Collaborator

also @GaballaGit dont worry about including DB models in swag, honestly might be preferable to not have them there. Those docs are most useful when you're creating a client, and clients don't have to interact with the dbmodels at all.

@GaballaGit
Copy link
Copy Markdown
Member Author

okay I can remove that quickly

@GaballaGit
Copy link
Copy Markdown
Member Author

Wait Josh do you know why we have the parseDependcy flag in the swag gen?

@TheJolman
Copy link
Copy Markdown
Collaborator

Wait Josh do you know why we have the parseDependcy flag in the swag gen?

yeah that was to parse sql.NullString or something like that, you can remove it if you're removing dbmodels from it

@GaballaGit
Copy link
Copy Markdown
Member Author

Okay cool, right now it with parseDep it grabs dbmodels. However, since dto hasn't been implemented outside of announcements the command errors out without it.

Copy link
Copy Markdown
Collaborator

@TheJolman TheJolman left a comment

Choose a reason for hiding this comment

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

sick!

@TheJolman TheJolman merged commit facc9a9 into main Apr 22, 2026
2 checks passed
@TheJolman TheJolman deleted the feat/Domain-Models branch April 22, 2026 20:23
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.

2 participants