Skip to content
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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

krasenyp
Copy link
Collaborator

@krasenyp 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.

@krasenyp krasenyp linked an issue Dec 1, 2022 that may be closed by this pull request
@krasenyp krasenyp force-pushed the jobs/post-creation branch 2 times, most recently from c956981 to b7270ab Compare December 2, 2022 07:12
embeds_one(:external, Erlef.Accounts.External, on_replace: :update)

belongs_to(:sponsor, Sponsor)
Copy link
Member

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?

Copy link
Collaborator Author

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.

lib/erlef/jobs.ex Outdated Show resolved Hide resolved

defp create_expired_at_column() do
query = """
ALTER TABLE job_posts
Copy link
Member

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?

Copy link
Collaborator Author

@krasenyp krasenyp Dec 5, 2022

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.

Copy link
Member

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?

lib/erlef/repo.ex Outdated Show resolved Hide resolved
@krasenyp krasenyp force-pushed the jobs/post-creation branch 2 times, most recently from e5562da to 8ca3454 Compare December 5, 2022 17:02
@krasenyp krasenyp force-pushed the jobs/post-creation branch 2 times, most recently from 19e8fd7 to b353be2 Compare December 14, 2022 14:18
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.
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.

Add ability to create, update, and delete job posts
2 participants