panoramix360 commented 10 months ago

This improves the Tags Page to have much more details on its table #396.

I moved some common methods between Stats <-> Tag to do the sorting on the Repo module, maybe we could create a new module for it, but let's see what you guys think.

Things missing

[x] Add tests for the new stuff
Sorting the virtual columns https://github.com/dwyl/mvp/issues/436

nelsonic commented 10 months ago

Dedicated issue for sorting/filtering: https://github.com/dwyl/mvp/issues/436

panoramix360 commented 10 months ago

hey @nelsonic and @LuchoTurtle!

Finally, I was able to test the files!

Can you guys review it?

Can you help me with this?

I'm having trouble testing the tags_live, it seems that the creation of tags/items is not producing results on the tests, this is strange since the other live tests work fine. When I add tags to the DB it seems that it doesn't get added for the tests to work.
I fixed the merge conflicts but I saw that you guys created some modifications regarding lists on the tags controller/view and this might need some work to the index live view of the tags.
I notice that the test files are being ignored by the formatter on .formatter.exs, is this the intended behavior?

I'll wait for your answers!

Thank you and sorry to take a while to create tests for these.

panoramix360 commented 10 months ago

@nelsonic

Forgot to mention that I saw that you commented some asserts on the stats_live tests and I fixed them! :D

codecov[bot] commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (b5c476f) 99.69% compared to head (74c584e) 99.70%. Report is 15 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #435 +/- ## ======================================= Coverage 99.69% 99.70% ======================================= Files 26 27 +1 Lines 659 677 +18 ======================================= + Hits 657 675 +18 Misses 2 2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

nelsonic commented 10 months ago

@panoramix360 code looks good. ✅ Ran it on my localhost and looks good. 👌

Added some style to the `Create Tag" Button:

Coverage at 98.5% should be relatively easy to get back up to 100. 💭

----------------
COV    FILE                                        LINES RELEVANT   MISSED
100.0% lib/api/item.ex                               218       56        0
100.0% lib/api/tag.ex                                101       24        0
100.0% lib/api/timer.ex                              152       40        0
100.0% lib/app/cid.ex                                 25        7        0
100.0% lib/app/color.ex                               90        1        0
100.0% lib/app/date_time_helper.ex                    18        5        0
100.0% lib/app/item.ex                               414       61        0
100.0% lib/app/item_tag.ex                            12        1        0
100.0% lib/app/list.ex                               272       46        0
100.0% lib/app/person.ex                               6        1        0
100.0% lib/app/stats.ex                               91       14        0
100.0% lib/app/tag.ex                                170       31        0
100.0% lib/app/timer.ex                              481       90        0
100.0% lib/app_web/controllers/auth_controller.       25        4        0
100.0% lib/app_web/controllers/init_controller.       44        8        0
100.0% lib/app_web/controllers/list_controller.       93       30        0
100.0% lib/app_web/controllers/tag_controller.e       70       22        0
 98.7% lib/app_web/live/app_live.ex                  569      161        2
100.0% lib/app_web/live/components/nav.ex              7        1        0
100.0% lib/app_web/live/components/table_compon       11        1        0
100.0% lib/app_web/live/stats_live.ex                117       31        0
 46.1% lib/app_web/live/tags_live.ex                  60       13        7
100.0% lib/app_web/router.ex                          51       10        0
100.0% lib/app_web/views/error_view.ex                59       12        0
100.0% lib/app_web/views/list_view.ex                 25        4        0
100.0% lib/app_web/views/nav_view.ex                  16        3        0
100.0% lib/app_web/views/table_component_view.e       19        3        0
  0.0% lib/app_web/views/tag_view.ex                   8        1        1
[TOTAL]  98.5%
----------------

Assign to me if you want me to work on the tests. 👌

panoramix360 commented 10 months ago

great @nelsonic! 😄

Can you help me test the tags_live? I just pinged you on a test case there.

Thank you!

I don't why, but

panoramix360 commented 9 months ago

hey! just noticed my last message got cut haha

@nelsonic did you have the time to take a look at the tests?

I just returned from vacation and will probably try to fix them again. They are on the tags_live_test.exs file.

panoramix360 commented 8 months ago

@nelsonic @LuchoTurtle

Happy New Year guys! :D 🥂

Glad to say that this PR is ready for review with the tests for the tag_live finally! Sorry for this delay, the end of the year was a little troublesome.

Ready to work on another issue! 😀

LuchoTurtle commented 8 months ago

Thanks for the amazing work @panoramix360 <3 I'm testing this and everything seems to be working perfectly!

Running mix c locally also works without a hitch. I'll take a quick look over the code and approve it :)

panoramix360 commented 8 months ago

hey @nelsonic and @LuchoTurtle

I just created the new section on the docs with these modifications, but I'm unable to push the branch on this repo.

Should I fork it to add a pull request there? I think that I had the permissions earlier for this.

Please, let me know what I need to do, thanks!

LuchoTurtle commented 8 months ago

@panoramix360 🤔 you should be able to at least create a PR. If you can't, maybe forking and creating a PR will be faster. If that's too much work, you can simply add the section you've written to this repo's BUILDIT.md, which also works :)

panoramix360 commented 8 months ago

great!

it's here!

https://github.com/dwyl/book/pull/100

LuchoTurtle commented 8 months ago

Thank you so much @panoramix360 ! Can you copy the text from https://github.com/dwyl/book/pull/100/files you've just created into the BUILDIT.md in this PR? Just for completeness :) I'll approve it after you commit it.

Last stretch! 🏃 Thank you so, so much, again! 🙏

panoramix360 commented 8 months ago

@LuchoTurtle, done!

nelsonic commented 8 months ago

"Merging can be performed automatically once the requested changes are addressed."

🤷‍♂️

panoramix360 commented 8 months ago

Strange, the requested changes were already addressed

nelsonic commented 8 months ago

Yeah, all seem to be resolved. Might need to manual override to merge. 💭

nelsonic commented 8 months ago

I think it's because @LuchoTurtle requested the changes so it's "pending reviewer" ...

nelsonic commented 8 months ago

I'm going to manual override as @LuchoTurtle is busy. 👌

dwyl / mvp

Tags Page with more details #435

Things missing

Codecov Report