KatjaGlassConsulting / ApprovalBundle

A Kimai Plugin to manage weekly approval workflow
MIT License
11 stars 14 forks source link

ApprovalBundle for Kimai 2 #25

Closed kevinpapst closed 6 months ago

kevinpapst commented 6 months ago

Hi Katja,

this PR brings the ApprovalBundle to the new major version of Kimai.

The changes took around ~14 hours and I has to adjust so many different things, while also fixing a couple of bugs on the fly ... I know this is a terrible PR to review, due to the size, sorry 🀷

Do you think we can incorporate that PR and release it under a new major version?

Edit 1: Ups, I just saw the existing v2 branch... so sorry... I wasn't aware it existed ... I did not use any code from there. Edit 2: I merged your latest changes and the new "overtime for all" report as well

Cheers Kevin

kevinpapst commented 6 months ago

ping @vitormattos and @sammaclennan - can you help testing this branch, to give Katja and myself some more confidence in the changes?

As I have written in the inital PR description, I did not see your WIP branch, which I am super sorry for 😒 but now it is too late and it seems we did the changes twice. To make it up to you: the bundle seems to work (at least in my Kimai 2 development environment).

KatjaGlassConsulting commented 6 months ago

Hi @kevinpapst,

thanks for this huge PR and also for enhancing the source. This enables me to learn more, so the source gets better and better. Give me a few days for the review - somehow these days are busy πŸŽ…. As we had been working on the update in the other branch - the review should be not too tough.

As I plan for a longer time to support both versions (my customer stays with v1, many others use and want v2) - I will maintain two versions in two branches - v1 with priority. I had been initially thinking of supporting the V2 in a branch and stay with main in V1. It might make sense to switch though. Can the marked place reference the plugin-support for both versions using two different links (one main branch, one feature branch)?

Thanks and have great Christmas Days!

kevinpapst commented 6 months ago

For the general workflow I would recommend:

We can link wherever you want from the store, the current link points to the README.

And for maintaining both branches I would recommend that you always work with feature branches and PRs. Then you can ping me and I will prepare the same PR for the main branch. Might be easier, if you have no v2 setup for testing.

And: Happy πŸŽ„ 🎁

KatjaGlassConsulting commented 6 months ago

Hi Kevin, yes I will do so. I already started working on this. I have a minimal V2 instance up and running and will continue some testing. The "cache" classes are also an excellent process to enhance performance on unique queries. I will take this over for v1 as well. Thanks also for the ping opportunity - as will continue to work on 1 for some time that's very much appreciated - I don't expect major updates anyway.

kevinpapst commented 6 months ago

The "cache" classes are also an excellent process to enhance performance on unique queries. I will take this over for v1 as well.

Definitely, could be used here pretty good. As you always know that the cache can be expired, as soon as someone submits a new approval request. Well, I already did some optimizations which were not strictly related to the v2 and thought that the PR wouldn't get easier to read with even more "unnecessary” changes 😁 but we can look at that together later on!

vitormattos commented 6 months ago

Hi @kevinpapst, Was this branch created from main or from kimai_v2_update? Sounds that you created your PR from main because have a lot of changes that I already made at kimai_v2_update

vitormattos commented 6 months ago

I found the answer here: https://github.com/KatjaGlassConsulting/ApprovalBundle/issues/17#issuecomment-1868444855

KatjaGlassConsulting commented 6 months ago

Apart from the strange URL as mentioned above and the opportunity to remove the number indicator on how many approvals are open, the PR is working perfectly fine in my environment with my data.

KatjaGlassConsulting commented 6 months ago

Thanks again for all your support here. The version 2 support is now available on the main branch and version 1 is available on bundle_for_kimai_v1. I have decided to go with ApprovalBundle version 1.x.x for kimai 1 support and version 2.x.x for kimai 2 support. I am going to work on performance updates in January and probably include some minor settings which I should be able to do for 1 and 2. If there is something more complex coming I will ping you. Thanks for this option.

Wish you happy holidays and a good start into the new year.

vitormattos commented 6 months ago

Maybe would more clear: stable-kimai-v1 or only stable1 and would be nice to have a CONTRIBUTING.md file explaining about this pattern or maybe a wiki page of this repo.

This could be solved doing a checkout of branch bundle_for_kimai_v1, creating the new branch name and pushing the new branch name. After this, deleting the bundle_for_kimai_v1 branch.

What you think about this @KatjaGlassConsulting ?

kevinpapst commented 6 months ago

Very strange, I did not receive any message about the PR being merged or your replies...

I think I pushed a couple of other changes after your merge.

Yeah, thank you Github πŸ€ͺ so I'll need to check and open a new PR^^

Thanks for merging @KatjaGlassConsulting πŸ‘

kevinpapst commented 6 months ago

Found the commits and created a new PR: https://github.com/KatjaGlassConsulting/ApprovalBundle/pull/28

vitormattos commented 6 months ago

Is this my PR: https://github.com/KatjaGlassConsulting/ApprovalBundle/pull/20

We made the same changes. You didn't saw my contributions because was merged by @KatjaGlassConsulting to branch kimai_v2_update. I think that @KatjaGlassConsulting merged using a rebase from my branch beucause I can see my commits at branch kimai_v2_update.

I think that the branch kimai_v2_update need to be deleted because sounds that is impossible to merge my contributions because will have a lot of conflicts and you already made all that I did at this PR.

But don't worry, the important is that now the code of this project was updated to be compatible with newest version of Kimai.

KatjaGlassConsulting commented 6 months ago

Hi @vitormattos, I will create a ignore/kimai_v2_update based on your branch as you branch shows very nicely the single steps to perform when migrating from 1 to 2. I very much appreciate you step-wise approach. Didn't had time to clean up the branches.

vitormattos commented 6 months ago

@KatjaGlassConsulting just ignore the branch kimai_v2_update for now. I made a work very similar to Kevin and will be near to impossible to recovery the work at branch kimai_v2_update.

Don't worry about this, is problems that could occur at our work as software engineer. We can create and destroy codes and we can don't use codes that we created and this isn't a problem. Is part of our work.

Only a good point that will be good is create a flow to contribute. If you want, I can create a draft PR about this creating a CONTRIBUTING.md file to we start a discussion over the suggestion of workflow. If you like, you can merge.

KatjaGlassConsulting commented 6 months ago

Hi @vitormattos, if you already have a skeleton for a contributing and could draft something, that would be nice yes. What I would like is that people open up an issue and always write there when they are working on something, so everyone can be aware.

The migration has been a super-big task which had not been working on for quite some time - as I am still in the V1 world, that's my focus and finding time is always a issue. So your and other contributions are very welcome. Unluckily it was taken up by multiples at the same/shortly after time. I would not expect something like this to happen again, especially as typically there are no major changes.

vitormattos commented 6 months ago

πŸ˜ƒ This is the power of open source projects! When we don't think that will receive contributions, we can receive a lot of contributions that will make us very happy and we can see that our work help people that wish to help us, be only sharing our project, contributing with documentation, suggestions, coding and even with financing.