-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
Add support for umami analytics in status page #5608
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
base: master
Are you sure you want to change the base?
Add support for umami analytics in status page #5608
Conversation
ffe03e8
to
6cd8931
Compare
6cd8931
to
afae736
Compare
It would be very neat if you could make the implementation a bit more generic, so that other analytics software can be used as well. Plausible, for example, uses an almost identical format for injecting your "tracker" inside a script tag. could easily be made generic imo. |
@NotAShelf Very nice idea, for now, I've tested my code and it fits my need to add umami tracking. I will probably open another issue and make another pr to make it more generic as the code truly is very similar to the google analytics pr I had seen.
I've found many more but these are the main ones I've found being used online. If there are any more, I invite anyone who uses them to list them in a comment so we can discuss adding them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty good.
I have left a comment about the migration/db-design and one how we could declutter this part of the UI.
Hey, just saw your comments @CommanderStorm, I will take them into account this week hopefully, can't promise anything. |
71761e9
to
a52341b
Compare
a52341b
to
e44ec55
Compare
@CommanderStorm I finished taking into account your remarks. I have honestly never used vue and it took me a while to get the select working, but it is tested and works as intended (automated tests and I tested it manually by connecting it to my own umami instance). |
2dd52c4
to
6bade1f
Compare
@CommanderStorm I finished making it generic and refactoring the code, the tests all pass, I will let you validate it whenever you feel like it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is my review. Took a bit longer, but hey exams..
We can merge it into V2.1, once these are adressed
1a2826c
to
8f480c7
Compare
8f480c7
to
28e288d
Compare
Hey All, sorry for the wait, I've had life happen to me for a few weeks. @CommanderStorm, I took into account your review, feel free to validate it or not, but I think this time, I didn't change the db init file when I shouldn't (sorry, I don't know why I changed it again when you had already told me not to, must've done it by habit xD). I also took your very understandable remark and use case into account @aldalen, I'll let you test out the proxying and validate that end as I only tested with umami and by giving the full script url with the updated code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work. Thanks for everything
Will be merged into V2.1 once master branches into this direction.
@aldalen I had you as a reviewer, but since you commented that it's okay for you, I suppose this can be merged. What do you think @CommanderStorm ? |
It will be merged into v2.1 as said above. |
https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md#can-i-create-a-pull-request-for-uptime-kuma
Tick the checkbox if you understand [x]:
Description
Fixes #5607
Fixes #5690
Fixes #2938
Type of change
Please delete any options that are not relevant.
Checklist
Screenshots (if any)
Please do not use any external image service. Instead, just paste in or drag and drop the image here, and it will be uploaded automatically.