Bugfix Git cleanup on draft/proposed branches when pushDraftsToGit is set on DB cache

[Catalin-Stratulat-Ericsson]

Bugfix Git cleanup on draft/proposed branches when pushDraftsToGit is set on DB cache

---

Description

• What changed: changed 2 if statements to also include pushDraftsToGit check during PR deletion
• Why it’s needed: resulted in leftover branches
• How it works: its an if statment

---

Related Issue(s)

• Closes/Fixes Draft/Proposed git branch cleanup on deletion #928

---

Type of Change

• Bug fix
• New feature
• Enhancement
• Refactor
• Documentation
• Tests
• Other: ________

---

Checklist

• Code follows project style guidelines
• Self-reviewed changes
• Tests added/updated
• Documentation added/updated
• All tests and gating checks pass

---

Testing Instructions (Optional)

1. launch locally with flag set and try deletion of PR after creation.
2. ensure git repo is removing branch

---

Additional Notes (Optional)

• Known issues:
• Further improvements: it seems approving PR's with this flag enabled breaks the process amongst other things. didn't include fix in this PR.
• Review notes:

---

AI Disclosure

• I have used AI in the creation of this PR.

If so, please describe how:

• Kiro was used to generate unit tests

[netlify]

✅ Deploy Preview for kpt-porch ready!

Name	Link
🔨 Latest commit	98c6d75
🔍 Latest deploy log	https://app.netlify.com/projects/kpt-porch/deploys/6a6773cb2bc20b00081b1e93
😎 Deploy Preview	https://deploy-preview-1136--kpt-porch.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[liamfallon]

It would be good to get a review from @rendre-greyling on this.

[rendre-greyling]

@liamfallon @Catalin-Stratulat-Ericsson LGTM - this is a known issue, but I did not realize that push drafts was enabled here.

I'll just leave a note here, there are some other issues regarding the push drafts to git implementation (both functional and performance related). I've got a refactor of this feature and the dbsync that I'm busy porting over, but some changes are needed to get it wired into the repo controller.

[efiacor]

We have some postponed tests that we should enable (just remove the "P" from PIt) for this change:

https://github.com/kptdev/porch/blob/main/test/e2e/crd/lifecycle_test.go#L204

https://github.com/kptdev/porch/blob/main/test/e2e/crd/lifecycle_test.go#L228

[Catalin-Stratulat-Ericsson]

@liamfallon @Catalin-Stratulat-Ericsson LGTM - this is a known issue, but I did not realize that push drafts was enabled here.

I'll just leave a note here, there are some other issues regarding the push drafts to git implementation (both functional and performance related). I've got a refactor of this feature and the dbsync that I'm busy porting over, but some changes are needed to get it wired into the repo controller.

thats good to hear @rendre-greyling. whilst looking into this i found a number of other issues that i didnt patch here, such as approves breaking down along other issues. i was going to raise another PR but its good to know you already have a refactor/fix. ill leave those to you :)

[Catalin-Stratulat-Ericsson]

We have some postponed tests that we should enable (just remove the "P" from PIt) for this change:

https://github.com/kptdev/porch/blob/main/test/e2e/crd/lifecycle_test.go#L204

https://github.com/kptdev/porch/blob/main/test/e2e/crd/lifecycle_test.go#L228

ahhhhh thats what i forgot. i remember seeing those tests when i first started looking into this but forgot about them. i can modify this PR to enable them. thanks for the reminder.

@efiacor should i remove the unit tests with this PR and enable those e2e ones or just leave as is?

[efiacor]

We have some postponed tests that we should enable (just remove the "P" from PIt) for this change:
https://github.com/kptdev/porch/blob/main/test/e2e/crd/lifecycle_test.go#L204
https://github.com/kptdev/porch/blob/main/test/e2e/crd/lifecycle_test.go#L228

ahhhhh thats what i forgot. i remember seeing those tests when i first started looking into this but forgot about them. i can modify this PR to enable them. thanks for the reminder.

@efiacor should i remove the unit tests with this PR and enable those e2e ones or just leave as is?

No, we can keep the existing ones also. Not sure if we need a similar e2e one for api.