-
-
Notifications
You must be signed in to change notification settings - Fork 47
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
Create, update, and delete a job post #430
base: main
Are you sure you want to change the base?
Conversation
krasenyp
commented
Dec 1, 2022
- I acknowledge my contribution to the website does not assert comparative or superlative differences of one product, project, company or individual over another.
c956981
to
b7270ab
Compare
embeds_one(:external, Erlef.Accounts.External, on_replace: :update) | ||
|
||
belongs_to(:sponsor, Sponsor) |
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.
Shouldn't this be a has_many(:members, Member)
on the Sponsor schema?
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.
@starbelly I suspect the common access pattern would be member -> sponsor
because we fetch the member from the database on every request and the member is the one performing the create/update/delete actions on the job posts.
|
||
defp create_expired_at_column() do | ||
query = """ | ||
ALTER TABLE job_posts |
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.
This is pretty cool and a TIL. Only concern is putting logic in the database. Some times that's the right tool for the job, but is there a reason why we would prefer this over doing this in the app when we insert or update, performance cost aside?
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.
@starbelly Everything in the database is more or less related to logic. Since the expiration date is a derived state, I think it's best to leave the owner of the data manage it. In this case it simplifies the application code because there's no need to think how is the record inserted. Is it inserted by Repo.insert
or a raw query. Also Postgres has very nice functions and operators for working with intervals.
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.
Postgres has very nice functions indeed 😁 However, expired_at at least to me is application logic (I definitely should have said application logic), which doesn't belong in the database. I can always play devil's advocate and argue why this might be better done by the databases. However, and for arguments sake if this syntax changes, becomes deprecated, etc. a migration in the database must be applied, what's more this feature requires upgrading the database to support it. If it was just a small operation in the application as part of the changeset, it would always work 😁
I provided the above merely for communication purposes (i.e., so I'm more clear about my rationale). That said, I'm not going to block on this one, as my feelings are not that strong and we do need upgrade the database 😄
On a related note, it looks like the a test is failing because of this change. Haven't dug into why other than I saw a syntax error. Wrong version of postgres used in workflow perhaps?
e5562da
to
8ca3454
Compare
19e8fd7
to
b353be2
Compare
b353be2
to
3dc4a4b
Compare
3dc4a4b
to
defbac4
Compare
This PR implements the ability for members to create, update and delete job posts. It introduces the following important changes: * Creates a `job_posts` database table. * Creates a `job_posts_hisory` database table for tracking updates on the `job_posts` table. * Creates a `Jobs` context to house the functionality related to job posts. * Creates a `Jobs.Post` Ecto schema. * Creates a `Jobs.PostHistoryEntry` Ecto schema. * Creates a `JobController` which allows a user to create, update and delete their own job posts. * Adds a basic authorization logic in the context and controller. * Adds a section in the basic member profile for managing job posts. * Adds notification logic after a post is created. This PR also introduces some minor changes which can be viewed as placeholders for later iterations: * Adds a top navigation entry for "Jobs". * Adds a job posts index page where approved job posts are listed. * Adds a mechanism for approving job posts by administrators. * Adds notification logic after a post is approved. * Associates members with sponsors.
defbac4
to
57a586e
Compare