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

Change REST Contract for statistics reports #75

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

Conversation

dimitrispie
Copy link

No description provided.

@@ -26,7 +26,30 @@ The sections below describe the parameters for a page view event. All other info
"targetType": "item"
}
```
## Report View Events endpoint
**GET /api/statistics/viewevents**
Copy link
Member

@artlowel artlowel Sep 5, 2019

Choose a reason for hiding this comment

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

I don't think this endpoint is the right place for statistics reports. Since you use it to POST one event at a time, you would expect to GET events with the same level of detail, and that's not very useful for a report. Reports are usually about a combination of a few aspects of a number of events at the same time. e.g. the most popular items in the repository, where we need to facet the view events on item id.

I envision reports will get a dedicated endpoint e.g. /api/statistics/reports that will return data in some json equivalent of a table format

Copy link
Author

Choose a reason for hiding this comment

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

I agree. That was my mistake to leave the endpoint as is. I will change it.

@dimitrispie dimitrispie changed the title Change contract for statistics reports Change REST Contract for statistics reports Sep 5, 2019
@tdonohue tdonohue changed the base branch from master to main July 13, 2020 20:45
@github-actions github-actions bot added the merge conflict PR has a merge conflict that needs resolution label Nov 30, 2023
Copy link

Hi @dimitrispie,
Conflicts have been detected against the base branch.
Please resolve these conflicts as soon as you can. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge conflict PR has a merge conflict that needs resolution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants