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

Check timestamps are defined? #19

Open
SimonLab opened this issue Nov 22, 2018 · 1 comment
Open

Check timestamps are defined? #19

SimonLab opened this issue Nov 22, 2018 · 1 comment
Labels
question Further information is requested

Comments

@SimonLab
Copy link
Member

SimonLab commented Nov 22, 2018

alog is checking that the schemas of the application have the minimum fields defined to allow the package to work:

Your Schema must have a key :deleted, with type :boolean and default false.

Your Schema must have a key :entry_id, with type :string

However it doesn't check that timestamps are defined.
timestamps are used in the queries to make sure to retrieve the correct item:

alog/lib/alog.ex

Lines 106 to 114 in 4cbf9d7

def get(entry_id) do
sub =
from(
m in __MODULE__,
where: m.entry_id == ^entry_id,
order_by: [desc: :inserted_at],
limit: 1,
select: m
)

Do we need to add a check for inserted_at and updated_at and if yes do we want to check that the type of these value is naive_datetime_usec to make sure that microseconds are saved in Postgres?

see https://hexdocs.pm/ecto/Ecto.Schema.html#module-the-datetime-types

@SimonLab SimonLab added the question Further information is requested label Nov 22, 2018
@Danwhy
Copy link
Member

Danwhy commented Nov 26, 2018

👍 Timestamps are necessary so we probably should require them the same way.

I don't think it's necessary that the type be naive_datetime_usec though. The timestamps are used to ensure we're fetching the most recent update of an item, and I don't think it's likely that two relevant updates would be performed within microseconds of each other. Saying that though, it would provide more of a guarantee of correct data, so maybe we should recommend it rather than require it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants