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.

Sign up for GitHub

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

Jump to bottom

Add missing timestamps to ecto schemas #2365

Merged
merged 2 commits into from
Feb 26, 2024
Merged

Add missing timestamps to ecto schemas #2365

merged 2 commits into from
Feb 26, 2024

Conversation

CDimonaco
Copy link
Member

Description

This pr adds the missing timestamps fields to all ecto schemas for minimal auditability.

How was this tested?

Automated tests, manual rollback and migration checking.

@CDimonaco CDimonaco added enhancement New feature or request tech debt elixir Pull requests that update Elixir code labels Feb 26, 2024
@CDimonaco CDimonaco self-assigned this Feb 26, 2024
@CDimonaco CDimonaco force-pushed the ecto_timestamps_missing branch 2 times, most recently from f721437 to 9cfe171 Compare February 26, 2024 11:49
arbulu89
arbulu89 reviewed Feb 26, 2024
View reviewed changes
Copy link
Contributor

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

Don't you need to update the views now to remove that?
Because we don't have it in the schemas

arbulu89
arbulu89 approved these changes Feb 26, 2024
View reviewed changes
Copy link
Contributor

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

It looks good.
My unique concern is the host table.
You added the values in the schema, but in the view we remove them.
I think you lack updating the view to not remove those values anymore

lib/trento_web/openapi/v1/schema/cluster.ex Show resolved Hide resolved
lib/trento_web/openapi/v1/schema/host.ex Show resolved Hide resolved
@CDimonaco CDimonaco merged commit 7cbed3d into main Feb 26, 2024
24 checks passed
@CDimonaco CDimonaco deleted the ecto_timestamps_missing branch February 26, 2024 13:42
@CDimonaco CDimonaco mentioned this pull request Feb 26, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arbulu89 arbulu89 arbulu89 approved these changes

Assignees

@CDimonaco CDimonaco

Labels
elixir Pull requests that update Elixir code enhancement New feature or request tech debt
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@CDimonaco @arbulu89