goinvo / staffplan-next-app

Next App for Staffplan
https://staffplan-next-app.vercel.app
1 stars 1 forks source link

Added visual view of hours in timeline for individuals #37

Closed MyStarrySpace closed 6 months ago

MyStarrySpace commented 6 months ago

You can now hover over cells to edit their values. Values are upserted when the mouse leaves a cell, so no need to manually submit edits. Updates are also done via state so their effects are immediately visible after updating.

vercel[bot] commented 6 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
staffplan-next-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 13, 2024 2:49pm
HankC138 commented 6 months ago

I'm not seeing that hover to edit effect, or the editing of workweeks held in state on the vercel preview? Is the vercel preview just not accurate maybe? I did solve the stale data in the workweek situation in my pull request from friday by updating the way the cache is handled by Apollo.

fermion commented 6 months ago

@MyStarrySpace did you ever by any chance see this error from the server locally? I saw a few failed upsert GraphQL calls and saw this in the logs:

21:11:44 web.1     | Validation failed: Cweek must be greater than or equal to 1
fermion commented 6 months ago

@HankC138 here's what I'm seeing locally:

https://github.com/goinvo/staffplan-next-app/assets/2804/241b8365-cdfb-4031-9fab-1c14e01a3385

It might be something with Vercel 🤷🏻 Actually, the Vercel builds may be getting CORS'd and may not function 🤔 I should look into that.

HankC138 commented 6 months ago

@HankC138 here's what I'm seeing locally:

Kapture.2024-03-10.at.21.16.04.mp4 It might be something with Vercel 🤷🏻 Actually, the Vercel builds may be getting CORS'd and may not function 🤔 I should look into that.

I just checked this locally and it doesn't seem to work. From what I'm seeing the new work weeks aren't making it to the upsert function at all. They are getting lost in the map somewhere. I'll have to have a deeper look at it soon as I get some time. I'll be in AZ starting tomorrow.

MyStarrySpace commented 6 months ago

Weird, it's working on my end and I was trying to investigate the error on Vercel and on your ends. I think I might have an old version of the upsert and I think that somewhere along the line, the schema might have changed. I'll look into it tomorrow

MyStarrySpace commented 6 months ago

As for the error, it seems that the error occurs when adding WorkWeeks to a week that has a cweek value of 0. I wasn't aware that these weeks started with an index of 1 and I calculated them a bit differently since the way that weeks were done using Corbin's approach was leading to some issues where some years would have duplicate week 0s, but I must have not tested upsetting the workWeek hours for cases where cweek=0

HankC138 commented 6 months ago

I'm unsure what you mean by duplicate weeks here. The cweek should never be zero as it starts at 1 And the way I calculated it was to generate a date in the format the backend handles using a date library. I should have some time in the early to late afternoon today to meet on solving this if you'd like?

Edit: just saw the previous comment Shirley about tomorrow. Ignore my previous ask about meeting up this afternoon.

fermion commented 6 months ago

@MyStarrySpace I haven't used date-fns previously, but it looks like getWeek may do what we want here? I'd maybe just check that it gives us the right value for the first week of the calendar year and some years gives us 53 for the last?