Skip to content

refactor: remove dead code and fix latent bugs found in a full codebase review #640

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.

Sign up for GitHub

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

Merged
nielsdrost7 merged 6 commits into from
Jul 18, 2026
Merged

refactor: remove dead code and fix latent bugs found in a full codebase review #640

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ca3bf6c
Merge remote-tracking branch 'upstream/develop' into develop
nielsdrost7 Jul 17, 2026
ea32d92
ran pint
nielsdrost7 Jul 17, 2026
22b38a6
improved .gitignore
nielsdrost7 Jul 17, 2026
b9a5348
improved .gitignore
nielsdrost7 Jul 17, 2026
866c1dc
refactor: remove dead code and fix latent bugs found in a full codeba…
nielsdrost7 Jul 18, 2026
da4c38c
Merge origin/develop (gitignore + pint hygiene) to reconcile fork drift
nielsdrost7 Jul 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,3 +111,7 @@ worktree-triage.md
/automation/test-honesty/vendor/
.claude/fable5/runtime/control.json
upd.sh
actual-real-resolved-issues.md
current-issues.md
merge-order.md
"saving some issues.md"

@coderabbitai coderabbitai Bot Jul 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== .gitignore context =="
nl -ba .gitignore | sed -n '108,124p'

echo
echo "== gitignore test =="
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
mkdir -p "$tmpdir/subdir"
: > "$tmpdir/saving some issues.md"
: > "$tmpdir/\"saving some issues.md\""
: > "$tmpdir/subdir/saving some issues.md"

git -C "$PWD" check-ignore -v -- "$tmpdir/saving some issues.md" "$tmpdir/\"saving some issues.md\"" "$tmpdir/subdir/saving some issues.md" || true

Repository: InvoicePlane/InvoicePlane-v2

Length of output: 231

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== .gitignore context =="
awk 'NR>=108 && NR<=124 { printf "%d:%s\n", NR, $0 }' .gitignore

echo
echo "== gitignore test =="
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
mkdir -p "$tmpdir/subdir"
: > "$tmpdir/saving some issues.md"
: > "$tmpdir/\"saving some issues.md\""
: > "$tmpdir/subdir/saving some issues.md"

# Show how Git matches the pattern from the repo root
git check-ignore -v -- "$tmpdir/saving some issues.md" "$tmpdir/\"saving some issues.md\"" "$tmpdir/subdir/saving some issues.md" || true

Repository: InvoicePlane/InvoicePlane-v2

Length of output: 613

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== .gitignore line =="
awk 'NR==117 { printf "%d:%s\n", NR, $0 }' .gitignore

echo
echo "== gitignore matching =="
printf '%s\n' \
  'saving some issues.md' \
  '"saving some issues.md"' \
  'subdir/saving some issues.md' \
| git check-ignore -v --stdin || true

echo
echo "== files with matching basename in repo =="
git ls-files | grep -F 'saving some issues.md' || true

Repository: InvoicePlane/InvoicePlane-v2

Length of output: 286

🌐 Web query:

gitignore double quotes literal characters pattern spaces quoted filenames documentation

💡 Result:

In gitignore files, double quotes do not have special meaning and are treated as literal characters [1]. Git patterns do not support enclosing filenames in double quotes to handle spaces or special characters [1]. To include spaces or special characters in a gitignore pattern, use a backslash () to escape them [2][3][4]. Specific rules regarding characters and spaces: 1. Spaces: Trailing spaces in a pattern are ignored unless they are escaped with a backslash [2][4]. To match a file or directory with a space, such as "my folder", you should write the pattern as "my\ folder" [1]. 2. Escaping: A backslash () can be used to escape any character to treat it as a literal [2][3]. For example, if you need to match a literal asterisk or exclamation mark, use * or ! [2][4]. 3. Double Quotes: If you place double quotes around a pattern, such as "file name", Git will look for a file that literally contains those double quotes in its name, rather than interpreting them as shell-style quotes for the filename [1].

Citations:

Remove the quotes from this ignore pattern. .gitignore treats double quotes as literal characters, so this only matches a filename that actually contains quotes. Use saving\ some\ issues.md instead.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.gitignore at line 117, Update the ignore pattern for saving some issues.md
by removing the literal double quotes and escaping the spaces so Git ignores the
intended filename.

4 changes: 1 addition & 3 deletions CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Modules/Core/Providers/ # All three Filament panel providers live here
| Module | Key models |
|--------|-----------|
| Core | User, Company, CompanyUser, TaxRate, Numbering, EmailTemplate, CustomField, Upload, Note, AuditLog, Setting, MailQueue |
| Clients | Relation (table: `relations`), Contact, Address, Communication, ClientCustom (`PK: client_custom_id`) |
| Clients | Relation (table: `relations`), Contact, Address, Communication |

@coderabbitai coderabbitai Bot Jul 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== CLAUDE.md ==\n'
nl -ba CLAUDE.md | sed -n '1,80p'

printf '\n== AGENTS.md ==\n'
nl -ba AGENTS.md | sed -n '120,150p'

printf '\n== .claude/skills/non-standard-pks/SKILL.md ==\n'
nl -ba .claude/skills/non-standard-pks/SKILL.md | sed -n '1,120p'

Repository: InvoicePlane/InvoicePlane-v2

Length of output: 223

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== CLAUDE.md ==\n'
sed -n '1,80p' CLAUDE.md | cat -n

printf '\n== AGENTS.md ==\n'
sed -n '120,150p' AGENTS.md | cat -n

printf '\n== .claude/skills/non-standard-pks/SKILL.md ==\n'
sed -n '1,120p' .claude/skills/non-standard-pks/SKILL.md | cat -n

printf '\n== exact string search ==\n'
rg -n "ClientCustom::\\$primaryKey = 'client_custom_id'|Import::\\$primaryKey = 'import_id'|non-standard-pks|primaryKey" .

Repository: InvoicePlane/InvoicePlane-v2

Length of output: 7990

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== CLAUDE.md around line 248 ==\n'
sed -n '236,256p' CLAUDE.md | cat -n

printf '\n== literal PK search ==\n'
rg -n -F "ClientCustom::$primaryKey = 'client_custom_id'" AGENTS.md .claude/skills/non-standard-pks/SKILL.md CLAUDE.md || true
rg -n -F "Import::$primaryKey = 'import_id'" AGENTS.md .claude/skills/non-standard-pks/SKILL.md CLAUDE.md || true

Repository: InvoicePlane/InvoicePlane-v2

Length of output: 1028

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== model definitions ==\n'
rg -n -F "class ClientCustom" Modules app database . || true
rg -n -F "class Import" Modules app database . || true

printf '\n== primary key declarations ==\n'
rg -n 'protected\s+\$primaryKey\s*=\s*'\''client_custom_id'\''' Modules app database . || true
rg -n 'protected\s+\$primaryKey\s*=\s*'\''import_id'\''' Modules app database . || true

Repository: InvoicePlane/InvoicePlane-v2

Length of output: 223

Keep the non-standard PK docs in sync. CLAUDE.md no longer lists ClientCustom/Import, but AGENTS.md and .claude/skills/non-standard-pks/SKILL.md still treat them as confirmed custom-PK models. Remove or update those entries so the guidance matches the current model set.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@CLAUDE.md` at line 28, Update the non-standard primary-key model guidance in
CLAUDE.md and the corresponding entries in AGENTS.md and
.claude/skills/non-standard-pks/SKILL.md so ClientCustom and Import are
consistently removed or reflect the current confirmed model set. Keep all three
documents synchronized.

| Invoices | Invoice, InvoiceItem, RecurringInvoice |
| Quotes | Quote, QuoteItem |
| Payments | Payment |
Expand Down Expand Up @@ -245,8 +245,6 @@ Invoice::observe(InvoiceObserver::class);

- `Company::$primaryKey` = `id` (standard); URL slug is `search_code` (10 chars, unique, e.g. `ivplv2`)
- `User::$timestamps = false` — no created_at/updated_at on users table
- `ClientCustom::$primaryKey = 'client_custom_id'` — non-standard PK
- `Import::$primaryKey = 'import_id'` — non-standard PK
- `Relation` model → table `relations` (not `customers`, not `clients`)
- Soft deletes on: Invoice, Quote (and their items)
- `BelongsToCompany` trait → adds `company()` BelongsTo, `scopeForCompany()`, and global scope
Expand Down
12 changes: 6 additions & 6 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ _stop = $(if $(STOP),--stop-on-failure)

# The core PHPUnit invocation (all optional flags appended).
_phpunit = APP_ENV=testing $(PHPUNIT) --configuration $(CONFIG) \
--exclude-group failing,troubleshooting $(_stop) $(_filter) $(_group) $(_suite)
--exclude-group failing,flaky,troubleshooting $(_stop) $(_filter) $(_group) $(_suite)

# The core artisan invocation.
_artisan = APP_ENV=testing $(PHP) artisan test --exclude-group failing,troubleshooting
_artisan = APP_ENV=testing $(PHP) artisan test --exclude-group failing,flaky,troubleshooting

.DEFAULT_GOAL := help

Expand All @@ -91,7 +91,7 @@ help:
# ── Full suite ────────────────────────────────────────────────────────────────

docker-test:
docker exec ivpldock-workspace-1 bash -c "cd /var/www/projects/ip2 && DB_HOST=mariadb php artisan test --exclude-groups=failing,troubleshooting"
docker exec ivpldock-workspace-1 bash -c "cd /var/www/projects/ip2 && DB_HOST=mariadb php artisan test --exclude-group=failing,flaky,troubleshooting"

## ─── Full suite ───────────────────────────────────────────────────────────────
test:
Expand Down Expand Up @@ -231,9 +231,9 @@ test-group:
#test-no-failing:
# $(_phpunit) --exclude-group failing

## Run all tests except 'troubleshooting' and 'failing' groups
## Run all tests except 'troubleshooting', 'failing', and 'flaky' groups
#test-stable:
# $(_phpunit) --exclude-group failing,troubleshooting
# $(_phpunit) --exclude-group failing,flaky,troubleshooting

# ── Coverage ──────────────────────────────────────────────────────────────────

Expand Down Expand Up @@ -304,7 +304,7 @@ artisan-bail:
ci:
APP_ENV=testing $(PHPUNIT) \
--configuration $(CONFIG) \
--exclude-group failing,troubleshooting
--exclude-group failing,flaky,troubleshooting \
--stop-on-failure \
--stop-on-error \
--cache-result-file /dev/null
Expand Down
31 changes: 0 additions & 31 deletions Modules/Clients/Database/Factories/AddressableFactory.php
View file
Open in desktop

This file was deleted.

40 changes: 0 additions & 40 deletions Modules/Clients/Database/Factories/CustomerFactory.php
View file
Open in desktop

This file was deleted.

8 changes: 0 additions & 8 deletions Modules/Clients/Events/RelationCreating.php
View file
Open in desktop

This file was deleted.

51 changes: 0 additions & 51 deletions Modules/Clients/Http/Requests/API/ClientAPIRequest.php
View file
Open in desktop

This file was deleted.

4 changes: 2 additions & 2 deletions Modules/Clients/Listeners/CustomerWasCreatedListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@

namespace Modules\Clients\Listeners;

use Modules\Clients\Models\Relation;
use Modules\Clients\Events\CustomerWasCreated;

class CustomerWasCreatedListener
{
public function __construct() {}

public function handle(Relation $event): void {}
public function handle(CustomerWasCreated $event): void {}
}
4 changes: 2 additions & 2 deletions Modules/Clients/Listeners/CustomerWasUpdatedListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@

namespace Modules\Clients\Listeners;

use Modules\Clients\Models\Relation;
use Modules\Clients\Events\CustomerWasUpdated;

class CustomerWasUpdatedListener
{
public function __construct() {}

public function handle(Relation $event): void {}
public function handle(CustomerWasUpdated $event): void {}
}
45 changes: 0 additions & 45 deletions Modules/Clients/Models/Addressable.php
View file
Open in desktop

This file was deleted.

17 changes: 0 additions & 17 deletions Modules/Clients/Models/ClientCustom.php
View file
Open in desktop

This file was deleted.

5 changes: 0 additions & 5 deletions Modules/Clients/Models/Contact.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,11 +123,6 @@ public function getPrimaryPhoneAttribute(): ?string
->first()?->contactable_value;
}

public function getCompanyNameAttribute()
{
return $this->company_id ? Company::query()->find($this->company_id)->company_name : null;
}

/*
|--------------------------------------------------------------------------
| Factory
Expand Down
17 changes: 0 additions & 17 deletions Modules/Clients/Models/CustomerNote.php
View file
Open in desktop

This file was deleted.

9 changes: 0 additions & 9 deletions Modules/Clients/Models/Relation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,6 @@ class Relation extends Model
| Relationships
|--------------------------------------------------------------------------
*/
public function attachments(): void
{
// return $this->morphMany(Attachment, 'attachable');
}

public function addresses(): MorphMany
{
return $this->morphMany(Address::class, 'addressable');
Expand Down Expand Up @@ -227,10 +222,6 @@ public function getEmailCcAttribute(): array
->all();
}

/*public function getPrimaryContactAttribute(): string
{
return mb_trim($this->primary_ontact?->first_name . ' ' . $this->primary_contact?->last_name);
}*/
/*
|--------------------------------------------------------------------------
| Scopes
Expand Down
19 changes: 0 additions & 19 deletions Modules/Clients/Observers/CustomerObserver.php
View file
Open in desktop

This file was deleted.

12 changes: 0 additions & 12 deletions Modules/Clients/Observers/RelationObserver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,4 @@ public function creating(Relation $relation): void
}
}
}

/*static::created(function ($client): void {
//event(new CustomerCreated($client));
});

static::saving(function ($client): void {
//event(new CustomerSaving($client));
});

static::deleted(function ($client): void {
//event(new CustomerDeleted($client));
});*/
}
3 changes: 0 additions & 3 deletions Modules/Clients/Providers/ClientsServiceProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@
use Modules\Clients\Observers\ContactObserver;
use Modules\Clients\Observers\RelationObserver;
use Modules\Core\Commands\GenerateObservers;
use Modules\Core\Models\Schedule;
use Modules\Quotes\Providers\EventServiceProvider;
use Modules\Quotes\Providers\RouteServiceProvider;
use Nwidart\Modules\Traits\PathNamespace;
use RecursiveDirectoryIterator;
use RecursiveIteratorIterator;
Expand Down
Loading
Loading