frontend: show Webhook History under Setting->Integration
This is part 2 of PR #3342 that implements a table under Settings -> Integration page displaying data from webhook_history table. See image below for demo.
Thank you for the PR!
How difficult would be to also present a list of builds (if any) submitted by each webhook?
Otherwise, I'm curious if we need to make this page private eventually. Opinions?
I'm curious if we need to make this page private eventually.
The whole page should already be private. Accessing https://copr.fedorainfracloud.org/coprs/g/copr/copr/integrations/ in a private window automatically redirects me to login
How difficult would be to also present a list of builds (if any) submitted by each webhook?
I would be careful here, so it doesn't start easy and then become annoying for large projects. What about going the other way around and in our current builds table or build details linking the webhook?
The whole page should already be private
And do we want it? Is there a reason to?
I mean, the question is "if we could present the NEW data publicly", not to make the access credentials "public" :-) wich would basically imply a completely new route.
in our current builds table or build details linking the webhook?
Or this! That's a nit, but I typically want to see all the reactions of Copr's automation to a particular webhook call, not vice versa ... (I'm personally able to find the info no matter what, but it would make user's/admin's life easier I bet).
Thank you for the update, the code is much simpler now :-)
The humanize_datetime filter works well but we already have time_ago to do the same thing, so adding it makes me a little uneasy. I realize that time_ago doesn't work in this case because it expects the input to be a timestamp, not datetime. But I'd rather modify it with something like:
--- a/frontend/coprs_frontend/coprs/filters.py
+++ b/frontend/coprs_frontend/coprs/filters.py
@@ -165,6 +165,9 @@ def time_ago(time_in, until=None):
"""
if time_in is None:
return " - "
+
+ if isinstance(time_in, datetime.datetime):
+ now = time_in
if until is not None:
now = datetime.datetime.fromtimestamp(until)
else:
instead of adding a new filter.
But I'd rather modify it with something like: if isinstance(time_in, datetime.datetime):
I can see you already do this workaround in another function. Maybe it would be even better to add a new migration to change WebhookHistory.timestamp from
timestamp = db.Column(DateTime(timezone=True), server_default=func.now())
to
created_on = db.Column(db.Integer, nullable=True)
so it is consistent with our other timestamps and therefore could use the same function without workarounds. What do you think?
Sounds good to me. From a maintenance and readability POV it would be better to stay consistent with how date and times are handled within the codebase. In hindsight, praiskup did mention a preference over UNIX timestamps, I see why.
In hindsight, praiskup did mention a preference over UNIX timestamps,
Hehe, I don't recall stating my preference - but indeed +1 from me for timestamps. The major "why?" benefit for me is that it stupid simple format and it has the same value on every single place on this planet at the same time :-) - it's up to the client/UI to convert it like:
$ date --date "@$(date +%s)"
pá 20. září 2024, 12:02:24 CEST
I'm not even against having the sub-second resolution.
Everything looks good to me. Can you please squash all the commits together into one, so that I can merge?