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

Feat: Improve Stats Page #397

Closed
5 tasks done
nelsonic opened this issue Jul 18, 2023 · 24 comments
Closed
5 tasks done

Feat: Improve Stats Page #397

nelsonic opened this issue Jul 18, 2023 · 24 comments
Assignees
Labels
discuss Share your constructive thoughts on how to make progress with this issue elixir Pull requests that update Elixir code enhancement New feature or enhancement of existing functionality help wanted If you can help make progress with this issue, please comment! priority-2 Second highest priority, should be worked on as soon as the Priority-1 issues are finished T2h Time Estimate 2 Hours technical A technical issue that requires understanding of the code, infrastructure or dependencies

Comments

@nelsonic
Copy link
Member

nelsonic commented Jul 18, 2023

At present the /stats page is quite basic: https://mvp.fly.dev/stats
image

We can add a lot more info/insight to this page.

Todo

  • Make the table sortable. e.g. using: https:/indgy/LittleBigTable - this makes sense with more interesting data.
  • When the person first joined (for authenticated people)
  • Total timers - the total amount of elapsed time the person has logged using timers in the MVP.
  • Latest item date (item.created_at)
  • "Me" - highlight the row for the person viewing the /stats page; make it personal to them.
@nelsonic nelsonic added enhancement New feature or enhancement of existing functionality help wanted If you can help make progress with this issue, please comment! T2h Time Estimate 2 Hours technical A technical issue that requires understanding of the code, infrastructure or dependencies priority-2 Second highest priority, should be worked on as soon as the Priority-1 issues are finished discuss Share your constructive thoughts on how to make progress with this issue elixir Pull requests that update Elixir code labels Jul 18, 2023
@panoramix360
Copy link
Collaborator

hey! :)

Can I take this one? Or it needs more refinement?

Please, point me to a priority-1 that has already been defined if not :)

@LuchoTurtle
Copy link
Member

Personally, I think you can get on with it. This issue and #396 seem to be plausible for @panoramix360 to develop.
Do you agree @nelsonic ?

Since we are using Tailwind and have a license/access to https://tailwindui.com/, if you find any of their components suitable, you can comment on the PR and we'll drop the code in it on your request so you can work on it, if you want 😄

@nelsonic
Copy link
Member Author

Indeed. I opened several issue in response to the superb feedback given by @panoramix360 on: #140 (comment) 💬
I'm especially keen on enhancements that don't create merge conflicts with #345
So yes, #397 (this issue) and #396 are perfect for @panoramix360 (or anyone else in the community to work on). 🙏

@panoramix360 panoramix360 self-assigned this Jul 19, 2023
@panoramix360
Copy link
Collaborator

panoramix360 commented Jul 19, 2023

I'll work on it then! Just assigned it to me if that's not a problem.

@nelsonic
Copy link
Member Author

Awesome. Thanks. If you need access to Tailwind UI LMK (email me: "#{firstname}@#{github_org_name}.com") 📧

@panoramix360
Copy link
Collaborator

panoramix360 commented Jul 22, 2023

I was looking through the components in Tailwind UI and it seems they already have the sortable headings:
https://tailwindui.com/components/application-ui/lists/tables#component-7b5a46e74e475708d966ca31716f1771
image

Do we have access to it on the paid subscription?

I want to send you an email to get access if possible, but I didn't understand your last message hehe

@panoramix360
Copy link
Collaborator

panoramix360 commented Jul 22, 2023

About the task when the person joined, since we are using AuthPlug, it seems that it's using Phoenix Sessions to login.

This means that we are not storing the user in our database, right?

So to get this information I changed the query to bring the first date of the first item created by the user.

Can you tell me if this assumption is correct? Thanks! :D

image

Another question, in this task:

  • Total timers - the total timers the person has.

The idea is to sum the timers, right?

@LuchoTurtle
Copy link
Member

@panoramix360

Do we have access to it on the paid subscription?

Yes, we do :) You need a license to get the code of those components, which was what @nelsonic was trying to say in #397 (comment). If you give him an e-mail, he'll get you on with it.

This means that we are not storing the user in our database, right?

In this mvp, we are not storing any information from the people that use it. Through auth_plug , we know which person is using the app by their id. For more context on how this id is created, you may check https://authdemo.fly.dev/people (and subsequently https:/dwyl/auth). To make long story short, when you sign up through our auth service (either by Github, Google or Microsoft), our auth assigns you your ID from the info given by the provider you signed in from.

So, our "first joined" assumption, although interesting, is not entirely accurate because they didn't necessarily join whenever they created the first item. However, because you don't really have access to this kind of information, I think it's a fair implementation (@nelsonic can correct me on this).

Total timers - the total timers the person has.

Currently, the total timers (meaning the amount of timer objects a person has created) are already being shown on the page. Though I wager that summing all the timers and showcasing it in a different column would be an interesting insight to see how much time a person has spent on all the tasks they've created so far.

Again, thanks a lot for being proactive, it's wholeheartedly appreciated! If I'm wrong on anything that I've said here, @nelsonic will promptly correct me 👌

@panoramix360
Copy link
Collaborator

panoramix360 commented Jul 22, 2023

hey! thank you @LuchoTurtle

My email is edited to prevent spam

Regarding the auth_plug, I figured it was something like this, so since we don't have enough information on our database, I don't know how we can show this information reliably. I know that my solution is not entirely correct.

We can also create a user table and store this info there as soon as the auth_plug answer with an OK after the authentication. But I'd have to look through auth_plug docs if it's possible.

About the timers, I figure it would be something like this then:
image

I'll format the time correctly, I'm showing in seconds, but we can show in minutes. But I wanted to show you the progress.

It's nice to contribute, I hope to contribute to other projects as well, especially the ones about learning. I was a professor at a University in the past (I teach Swift/iOS and React Native :D), so I really like this education focus that you guys are doing.

I'll wait for @nelsonic thoughts as well, but thank you for that :)

@nelsonic
Copy link
Member Author

Hi @panoramix360, thanks for following up. All/always good questions. ❤️
You have an invite to join our TailwindUI; check your email. 📧
Then remove your email from the previous comment to avoid spam. 🙅

As @LuchoTurtle says, our goal is to store zero personally identifiable information in the MVP (or App) and just store the bare minimum data required to interact with the App ...
We consider the "first joined" date to be when they created their first item.
Otherwise they only authenticated and didn't actually interact with the MVP/App. 🥇

As Lucho has said, the the "total timers" is the amount of time as opposed to the count of timers. I have made it clearer in the OP. It's the elapsed time.

All contributions are very welcome. If you create a git branch within the org instead of an Fork then your commits will run on the GitHub Actions CI each time you git push. :shipit:

We are huge fans of sharing all of our learning so that anyone can follow along regardless of where/who they are. Yes, even if that means we are "training" ClosedAI ... 🙄 If it means that people (assisted by LLMs) write better code, it's all good. 👌

@panoramix360
Copy link
Collaborator

hey! Thank you for the TailwindUI access, I'll begin the implementation of the sorted columns :)

Understood and agreed about storing zero info on the MVP. So I'll consider the first created item as the first joined, it's already implemented then!

Thank you for the clarification of the elapsed time! I implemented that already, I'll show it in the following format 00:00:00.

Nice! I created a branch inside MVP for this current issue, I'll take a look at the other projects as well later :)

I'm seeing that you guys are creating some projects about Flutter, this interests me as well, I know a lot about the Mobile env like Android/iOS and even React Native, so it can be something that I can contribute.

Good know about sharing our learning, I really like that too!!!

@panoramix360
Copy link
Collaborator

@nelsonic and @LuchoTurtle

Take a look at the elapsed time:
image
image

I used the :humanized format on the Timex library.

I believe that's really nice :)

@nelsonic
Copy link
Member Author

nelsonic commented Jul 23, 2023

Good shout on using Timex.Format.Duration.Formatters.Humanized to format the time. 👌
Already a dependency:

mvp/mix.exs

Line 63 in 4a04f86

{:timex, "~> 3.7"},

So definitely use it. ✅

@nelsonic
Copy link
Member Author

Derp. didn't mean to close it. Some how clicked the wrong button. 🤦‍♂️
Thanks again for looking at this. 🙏

@nelsonic nelsonic reopened this Jul 23, 2023
@panoramix360
Copy link
Collaborator

panoramix360 commented Jul 31, 2023

hey!

I'm finishing the sorting mechanism and it's looking great!

I decided to create a Table LiveComponent that can be used in other places on the application as well! I hope that you guys like the idea/code. It's my first iteration of creating a LiveComponent so it was nice to learn.

So I extracted the template and files to a separate place that can be reused.

And the sorting will be triggered when a column is clicked passing the sorting column as a LiveView event for the metrics to be sorted out.

I need more data to test this sorting on the database, but I can't log in locally, is there something that I need to do related to the AuthPlug to run this? Or is not possible to run locally?

@LuchoTurtle
Copy link
Member

Awesome!

Can you expand on why you can't log in? Maybe you don't have the env variables properly set. Give https:/dwyl/auth_plug#2-get-your-auth_api_key- a read, it might help you.

However, if you really just want to test if the sorting works, maybe it's simpler to just create an array in the socket assigns with mock data and show it on the view and see if everything works. 💭

@panoramix360
Copy link
Collaborator

hey @LuchoTurtle :D

I was doing that (the mocking thing) but in regards to the sorting, this doesn't work, the sorting is being done on the Ecto/Database layer.

I'll take a look at the link that you sent.

In the meantime, I'll try to generate fake data on the database as well.

@panoramix360
Copy link
Collaborator

Do you know if the .env.sample of the project is pointing to a real fly.dev application? I don't think so, right? hehe

@LuchoTurtle
Copy link
Member

It's not :P

In practicality, you'll probably only need to change the AUTH_API_KEY inside env and run source .env so the env variables are available.

To get your own AUTH_API_KEY, just follow the link I've sent https:/dwyl/auth_plug#2-get-your-auth_api_key-. You basically just create a New App and use the AUTH_API_KEY given to you.

After these, if you run locally, you should have auth properly working on your local project :)

@panoramix360
Copy link
Collaborator

I managed to add items by hand to the database to make some tests.

I'll look into the auth later :)

The sorting is working :D I just need to create the Tailwind look now on the columns.

@nelsonic
Copy link
Member Author

@panoramix360 if you push a branch and open a PR we can help with any parts you don't have time for. 👌

@panoramix360
Copy link
Collaborator

panoramix360 commented Jul 31, 2023

Done! I opened a PR #399 so you guys can already review the code.

The only things missing are:

  • Let the user click on the Column title and change the ordering by ASC or DESC.
  • Add tests to the new code

@nelsonic
Copy link
Member Author

https://mvp.fly.dev/stats
image

or incognito:
image

Thanks again @panoramix360 🎉

@LuchoTurtle
Copy link
Member

Closing this since it's been completed 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Share your constructive thoughts on how to make progress with this issue elixir Pull requests that update Elixir code enhancement New feature or enhancement of existing functionality help wanted If you can help make progress with this issue, please comment! priority-2 Second highest priority, should be worked on as soon as the Priority-1 issues are finished T2h Time Estimate 2 Hours technical A technical issue that requires understanding of the code, infrastructure or dependencies
Projects
Status: Done
Development

No branches or pull requests

3 participants