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

Activity log endpoint #2739

Merged
merged 13 commits into from
Jul 4, 2024
Merged

Activity log endpoint #2739

merged 13 commits into from
Jul 4, 2024

Conversation

gagandeepb
Copy link
Contributor

@gagandeepb gagandeepb commented Jul 3, 2024

Description

Implements an endpoint for consuming the activity log.
Filtering/pagination is deferred for later work.
No additional docs needed.

Did you add the right label?

Remember to add the right labels to this PR.

  • DONE

How was this tested?

Describe the tests that have been added/changed for this new behavior.

Unit tests

  • DONE

Did you update the documentation?

Remember to ask yourself if your PR requires changes to the following documentation:

Add a documentation PR or write that no changes are required for the documentation.

  • DONE

@gagandeepb gagandeepb added enhancement New feature or request elixir Pull requests that update Elixir code labels Jul 3, 2024
@gagandeepb gagandeepb requested review from nelsonkopliku and removed request for nelsonkopliku July 3, 2024 12:36
@gagandeepb gagandeepb marked this pull request as ready for review July 3, 2024 13:05
lib/trento_web/views/v1/activity_log_view.ex Outdated Show resolved Hide resolved
lib/trento_web/router.ex Show resolved Hide resolved
Copy link
Member

@nelsonkopliku nelsonkopliku left a comment

Choose a reason for hiding this comment

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

Good Job!

Screenshot from 2024-07-04 08-49-50

I don't have a strong opinion on /activity_log vs /activity_log/entries.

lib/trento_web/views/v1/activity_log_view.ex Outdated Show resolved Hide resolved
@@ -38,6 +37,12 @@ defmodule Trento.ActivityLog do
end
end

@spec list_activity_log() :: list(ActivityLog.t())
def list_activity_log do
# This will be made filterable/paginatable in a later PR
Copy link
Member

Choose a reason for hiding this comment

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

issue: Avoid comments on future implementations

Suggested change
# This will be made filterable/paginatable in a later PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My intention here was to motivate/justify the use of Repo.all in the following line, which is not very usual. In a way this implementation is partial and I was only drawing attention to this fact.

@gagandeepb gagandeepb force-pushed the activity-log-endpoint branch from c75d8d8 to 9acb3bb Compare July 4, 2024 10:05
@gagandeepb gagandeepb merged commit bc874f1 into main Jul 4, 2024
26 checks passed
@gagandeepb gagandeepb deleted the activity-log-endpoint branch July 4, 2024 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
elixir Pull requests that update Elixir code enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

3 participants