[IP-370]: invoice line numbering

[nielsdrost7]

Summary by CodeRabbit

• New Features

  • Added a company setting to show position numbers on invoice line items.
  • Invoice forms and PDFs now display optional line-item position numbers.
  • Added a Company Settings entry for easier access.

• Bug Fixes

  • Company switching now prevents access to companies the signed-in user does not belong to.
  • Recent expenses, payments, projects, and tasks now appear in consistent newest-first order.

• Documentation

  • Updated testing guidance to use Docker, MariaDB, and the recommended Artisan test command.

• Tests

  • Added coverage for invoice settings and company-access validation.

[coderabbitai]

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details

Not Found - https://docs.github.com/rest/issues/comments#update-an-issue-comment

[coderabbitai]

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details

Not Found - https://docs.github.com/rest/issues/comments#update-an-issue-comment

[coderabbitai]

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details

Not Found - https://docs.github.com/rest/issues/comments#update-an-issue-comment

[coderabbitai]

Actionable comments posted: 13

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

docker-compose.yml (1)

69-69: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Pin the local MariaDB image to match CI.

docker-compose.yml uses the floating mariadb image, while CI uses mariadb:10.11. Pin the compose service so local tests run against the same minor MariaDB version.

• docker-compose.yml#L69-L69: pin the image to mariadb:10.11.
• CLAUDE.md#L198-L201: also update this from Mariadb 11 to MariaDB 10.11.

🤖 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 `@docker-compose.yml` at line 69, Pin the MariaDB service image in
docker-compose.yml at lines 69-69 to mariadb:10.11, matching CI. Update the
MariaDB version reference in CLAUDE.md at lines 198-201 from MariaDB 11 to
MariaDB 10.11.

🤖 Prompt for all review comments with 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.

Inline comments:
In `@CLAUDE.md`:
- Around line 202-204: Update the fenced code block containing the docker
compose command to specify the bash language identifier, preserving the command
unchanged.

In `@docker-compose.yml`:
- Around line 78-84: Add a non-destructive bootstrap path for the existing
MariaDB volume so `invoiceplane_test` is created with `CREATE DATABASE IF NOT
EXISTS` without requiring volume deletion. Update the database initialization
configuration around the `/docker-entrypoint-initdb.d` mount and provide a
command or service workflow developers can run against the existing database
container.
- Around line 60-61: Update the docker-compose service dependency for cli so db
has a MariaDB readiness healthcheck, and configure cli’s depends_on to require
db’s service_healthy condition instead of using short-form dependency syntax.
Preserve the existing startup relationship while ensuring cli starts only after
database connectivity is ready.
- Around line 38-41: Remove the explicit --exclude-group failing,troubleshooting
option from the recommended PHPUnit commands in docker-compose.yml (lines
38-41), .github/DOCKER.md (lines 51-60), AGENTS.md (lines 13-15), CLAUDE.md
(lines 198-207), and README.md (lines 242-251). Leave the commands using the
configured PHPUnit exclusions without overriding them.

In `@docker-resources/mariadb/init/01-create-test-db.sql`:
- Around line 3-5: Update the initialization comment to replace
vendor/bin/phpunit with docker compose run --rm cli php artisan test, preserving
the existing explanation that the command runs tests against the dedicated
MariaDB database.

In `@Modules/Core/Providers/CompanyPanelProvider.php`:
- Line 250: Update the NavigationItem::make label for Company Settings to use
trans('ip.settings') instead of the raw string, preserving the existing
navigation item structure and following the required trans()
internationalization helper.
- Around line 250-253: Add a visibility condition to the manually created
Company Settings NavigationItem in the navigation definition, reusing the same
Permission::MANAGE_COMPANY_SETTINGS predicate as the user-menu settings action.
Keep the existing URL and active-state behavior unchanged, and ensure users
without that permission do not see the item.

In `@Modules/Core/Services/UserService.php`:
- Line 149: Update the condition in the user-company membership check to use
PSR-12 spacing by removing the space between the opening parenthesis and the
negation operator in the if statement, while preserving the existing logic.

In `@Modules/Core/Tests/Feature/UserProfileTest.php`:
- Around line 118-128: Ensure the tenant-switch assertion in the test method
it_sets_the_tenant_and_redirects_to_the_target_dashboard_when_switching is
executed automatically in CI rather than only through the manually triggered
flaky-group workflow. Update the relevant PHPUnit workflow or grouping
configuration to isolate and reliably run this authorization coverage on pull
requests, while preserving the existing assertion.

In `@Modules/Core/Tests/Unit/Services/UserServiceTest.php`:
- Around line 10-17: Update UserServiceTest to extend AbstractTestCase instead
of AbstractAdminPanelTestCase, and adjust the imported base-class symbol
accordingly. Keep the existing test traits and attributes unchanged unless the
test specifically requires admin-panel integration.
- Around line 28-53: Add the PHPUnit #[Group('security')] attribute to the
UserService authorization tests, either at the test class level or on both
assertBelongsToCompany test methods:
it_allows_a_user_to_switch_to_a_company_they_belong_to and
it_refuses_to_switch_to_a_company_the_user_does_not_belong_to.

In
`@Modules/Invoices/Filament/Company/Resources/Invoices/Schemas/InvoiceForm.php`:
- Around line 316-331: Update the position calculation around the current
repeater row logic to identify the row by its repeater key or state path rather
than product_id. Ensure duplicate-product rows, null-product new rows, and
reordered rows each resolve to their own current position, and add coverage for
these cases.
- Line 165: Update the invoice item grid configuration around
getPositionNumberSchema() so enabling show_line_item_position_numbers keeps all
seven one-column fields, including subtotal, on a single row. Increase the Grid
capacity from 6 to 7, or equivalently reduce subtotal’s column span without
changing other item-field behavior.

---

Outside diff comments:
In `@docker-compose.yml`:
- Line 69: Pin the MariaDB service image in docker-compose.yml at lines 69-69 to
mariadb:10.11, matching CI. Update the MariaDB version reference in CLAUDE.md at
lines 198-201 from MariaDB 11 to MariaDB 10.11.

🪄 Autofix (Beta)

❌ Autofix failed (check again to retry)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 058c8954-b5c8-4245-8b9a-eed1b04f578b

📥 Commits

Reviewing files that changed from the base of the PR and between decacfa and 1b59249.

📒 Files selected for processing (26)

• .claude/skills/test-gaps/SKILL.md
• .github/DOCKER.md
• .github/workflows/phpunit.yml
• AGENTS.md
• CLAUDE.md
• Makefile
• Modules/Core/Filament/Company/Pages/CompanySettings.php
• Modules/Core/Filament/Company/Pages/MyCompanies.php
• Modules/Core/Models/Company.php
• Modules/Core/Models/Setting.php
• Modules/Core/Providers/CompanyPanelProvider.php
• Modules/Core/Services/UserService.php
• Modules/Core/Tests/Feature/CompanySettingsTest.php
• Modules/Core/Tests/Feature/UserProfileTest.php
• Modules/Core/Tests/Unit/Services/UserServiceTest.php
• Modules/Expenses/Filament/Company/Widgets/RecentExpensesWidget.php
• Modules/Invoices/Filament/Company/Resources/Invoices/Schemas/InvoiceForm.php
• Modules/Invoices/resources/views/pdf/invoice.blade.php
• Modules/Payments/Filament/Company/Widgets/RecentPaymentsWidget.php
• Modules/Projects/Filament/Company/Widgets/RecentProjectsWidget.php
• Modules/Projects/Filament/Company/Widgets/RecentTasksWidget.php
• README.md
• docker-compose.yml
• docker-resources/mariadb/init/01-create-test-db.sql
• docker-resources/php-cli/Dockerfile
• resources/lang/en/ip.php

[coderabbitai]

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

Specify the fenced-code language.

Add bash to this fenced code block. The current block fails markdownlint rule MD040.

🧰 Tools

🪛 markdownlint-cli2 (0.23.1)

[warning] 202-202: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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` around lines 202 - 204, Update the fenced code block containing
the docker compose command to specify the bash language identifier, preserving
the command unchanged.

Source: Linters/SAST tools

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Do not override the configured PHPUnit exclusion groups.

CI documents that an explicit --exclude-group option replaces the exclusions in phpunit.xml. These commands exclude only failing,troubleshooting, so they re-enable the configured flaky group and no longer match CI.

• docker-compose.yml#L38-L41: remove --exclude-group failing,troubleshooting from the recommended command.
• .github/DOCKER.md#L51-L60: remove the explicit exclusion option.
• AGENTS.md#L13-L15: remove the explicit exclusion option.
• CLAUDE.md#L198-L207: remove the explicit exclusion option.
• README.md#L242-L251: remove the explicit exclusion option.

📍 Affects 5 files

• docker-compose.yml#L38-L41 (this comment)
• .github/DOCKER.md#L51-L60
• AGENTS.md#L13-L15
• CLAUDE.md#L198-L207
• README.md#L242-L251

🤖 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 `@docker-compose.yml` around lines 38 - 41, Remove the explicit --exclude-group
failing,troubleshooting option from the recommended PHPUnit commands in
docker-compose.yml (lines 38-41), .github/DOCKER.md (lines 51-60), AGENTS.md
(lines 13-15), CLAUDE.md (lines 198-207), and README.md (lines 242-251). Leave
the commands using the configured PHPUnit exclusions without overriding them.

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Wait for MariaDB readiness before starting cli.

Short-form depends_on only starts db; it does not wait until MariaDB accepts connections. A fresh docker compose run --rm cli php artisan test can fail before migrations or tests connect. Add a db health check and use condition: service_healthy for cli.

🤖 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 `@docker-compose.yml` around lines 60 - 61, Update the docker-compose service
dependency for cli so db has a MariaDB readiness healthcheck, and configure
cli’s depends_on to require db’s service_healthy condition instead of using
short-form dependency syntax. Preserve the existing startup relationship while
ensuring cli starts only after database connectivity is ready.

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Do not require docker compose down -v to add the test database.

Existing database volumes skip /docker-entrypoint-initdb.d, so they do not receive invoiceplane_test. The documented recovery deletes the developer database volume. Provide a non-destructive bootstrap command, such as CREATE DATABASE IF NOT EXISTS invoiceplane_test, for existing volumes.

🤖 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 `@docker-compose.yml` around lines 78 - 84, Add a non-destructive bootstrap
path for the existing MariaDB volume so `invoiceplane_test` is created with
`CREATE DATABASE IF NOT EXISTS` without requiring volume deletion. Update the
database initialization configuration around the `/docker-entrypoint-initdb.d`
mount and provide a command or service workflow developers can run against the
existing database container.

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use the supported test command in this initialization comment.

vendor/bin/phpunit is documented elsewhere in this PR as unreliable for Livewire form tests. Replace it with docker compose run --rm cli php artisan test so this comment matches the supported test path.

🤖 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 `@docker-resources/mariadb/init/01-create-test-db.sql` around lines 3 - 5,
Update the initialization comment to replace vendor/bin/phpunit with docker
compose run --rm cli php artisan test, preserving the existing explanation that
the command runs tests against the dedicated MariaDB database.

[coderabbitai]

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 3 \
  --glob 'phpunit.xml*' \
  --glob '*.yml' \
  --glob '*.yaml' \
  '(artisan test|phpunit|exclude-group|--group|flaky)' \
  .

Repository: InvoicePlane/InvoicePlane-v2

Length of output: 1807

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== tracked workflow/config files =="
git ls-files '*.yml' '*.yaml' '*.xml' | sed -n '1,120p'

echo
echo "== phpunit.xml =="
cat -n phpunit.xml | sed -n '1,80p'

echo
echo "== workflow references to test/command group/exclude =="
git ls-files '*.yml' '*.yaml' | xargs -r rg -n -C 4 \
  '(php artisan test|vendor/bin/phpunit|phpunit|test:unit|test:feature|exclude-group|--group|flaky|failed|re-run|workflow_dispatch|schedule)' \
  || true

echo
echo "== UserProfileTest flaky group context =="
cat -n Modules/Core/Tests/Feature/UserProfileTest.php | sed -n '1,180p'

Repository: InvoicePlane/InvoicePlane-v2

Length of output: 41159

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== phpunit workflow =="
cat -n .github/workflows/phpunit.yml | sed -n '1,100p'

echo
echo "== smoke workflow =="
cat -n .github/workflows/smoke.yml | sed -n '1,100p'

echo
echo "== phpunit smoke xml =="
cat -n phpunit.smoke.xml | sed -n '1,100p'

echo
echo "== automatic workflow triggers =="
python3 - <<'PY'
from pathlib import Path
import re
for p in sorted(Path('.github/workflows').glob('*.yml')):
    text=p.read_text()
    if re.search(r'pull_request:|push:|workflow_dispatch:|schedule:', text):
        print(p)
        for line in text.splitlines()[:40]:
            if re.search(r'(pull_request:|push:|workflow_dispatch:|schedule:|name:)', line):
                print(line)
PY

Repository: InvoicePlane/InvoicePlane-v2

Length of output: 8461

---

Run this tenant-switch assertion in an automatic CI job.

.github/workflows/phpunit.yml has the only PHPUnit coverage for the flaky group, but it is workflow_dispatch-only and currently excludes flaky via phpunit.xml. The automatic smoke job runs only --group smoke, so pull requests are not covered for this it_sets_the_tenant_and_redirects_to_the_target_dashboard_when_switching test. Move the assertion into an automatic required job, isolate it to run reliably, or restore this authorization coverage without leaving it to manual-only CI.

🤖 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 `@Modules/Core/Tests/Feature/UserProfileTest.php` around lines 118 - 128,
Ensure the tenant-switch assertion in the test method
it_sets_the_tenant_and_redirects_to_the_target_dashboard_when_switching is
executed automatically in CI rather than only through the manually triggered
flaky-group workflow. Update the relevant PHPUnit workflow or grouping
configuration to isolate and reliably run this authorization coverage on pull
requests, while preserving the existing assertion.

[coderabbitai]

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

Use the non-panel test base class.

UserServiceTest does not test an admin-panel flow. Extend AbstractTestCase, or move this test to Feature if it requires panel integration.

As per coding guidelines, “Extend AbstractTestCase for pure unit tests, AbstractAdminPanelTestCase for admin panel tests, and AbstractCompanyPanelTestCase for company panel tests.”

🤖 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 `@Modules/Core/Tests/Unit/Services/UserServiceTest.php` around lines 10 - 17,
Update UserServiceTest to extend AbstractTestCase instead of
AbstractAdminPanelTestCase, and adjust the imported base-class symbol
accordingly. Keep the existing test traits and attributes unchanged unless the
test specifically requires admin-panel integration.

Source: Coding guidelines

[coderabbitai]

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate test file =="
fd -a 'UserServiceTest.php' . || true

echo "== inspect header and relevant methods =="
file="$(fd 'UserServiceTest.php' . | head -n 1)"
if [ -n "${file:-}" ]; then
  wc -l "$file"
  sed -n '1,140p' "$file" | cat -n
fi

echo "== search Group attributes in Modules tests =="
rg -n 'Group\(' Modules || true

echo "== check PHPUnit group config references =="
fd -a 'phpunit\.xml|phpunit\.php|phpunit\.xml\.dist' . | while read -r f; do
  echo "-- $f"
  sed -n '1,220p' "$f"
done

Repository: InvoicePlane/InvoicePlane-v2

Length of output: 40906

---

Add a PHPUnit group for the authorization tests.

Add #[Group('security')] at the class level or on both it_allows_a_user_to_switch_to_a_company_they_belong_to() and it_refuses_to_switch_to_a_company_the_user_does_not_belong_to() methods.

🤖 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 `@Modules/Core/Tests/Unit/Services/UserServiceTest.php` around lines 28 - 53,
Add the PHPUnit #[Group('security')] attribute to the UserService authorization
tests, either at the test class level or on both assertBelongsToCompany test
methods: it_allows_a_user_to_switch_to_a_company_they_belong_to and
it_refuses_to_switch_to_a_company_the_user_does_not_belong_to.

Source: Coding guidelines

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="Modules/Invoices/Filament/Company/Resources/Invoices/Schemas/InvoiceForm.php"
if [ ! -f "$file" ]; then
  echo "Missing file: $file"
  exit 0
fi

echo "== file size =="
wc -l "$file"

echo "== relevant schema outline/search =="
rg -n "function getPositionNumberSchema|self::getPositionNumberSchema|InvoiceFormItem|invoiceItems|Columns|Row|grid|TextInput|TextSelect|Select|Position|position" "$file" -C 3

echo "== targeted lines 120-190 =="
sed -n '120,190p' "$file" | nl -ba -v120

echo "== targeted section 1-240 =="
sed -n '1,240p' "$file" | nl -ba -v1

Repository: InvoicePlane/InvoicePlane-v2

Length of output: 9888

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="Modules/Invoices/Filament/Company/Resources/Invoices/Schemas/InvoiceForm.php"

echo "== columns / columnSpan occurrences in file =="
grep -nE "Columns::make\(|columnSpan\(|Grid::make\(|getPositionNumberSchema|position_number|product_id|Quantity|Price|Discount|Subtotal" "$file" || true

echo "== rows 152-205 =="
sed -n '152,205p' "$file"

echo "== rows 297-334 =="
sed -n '297,334p' "$file"

python3 - <<'PY'
from pathlib import Path
text = Path("Modules/Invoices/Filament/Company/Resources/Invoices/Schemas/InvoiceForm.php").read_text()
start = text.index("                ->schema([\n                        Repeater::make('invoiceItems'")
grid_start = text.index("                        Grid::make(6)", start)
grid_end = text.index("                        ],", grid_start)
grid = grid_start[0]
grid_code = text[grid_start:grid_end]
components = []
for needle in [
    "self::getPositionNumberSchema()",
    "TextInput::make('quantity')",
    "TextInput::make('price')",
    "TextInput::make('discount')",
    "TextInput::make('subtotal')",
    "TextInput::make('tax')",
    "Select::make('product_id')",
    "TextInput::make('product_name')",
    "TextInput::make('tax_id')",
]:
    idx = grid_code.find(needle)
    components.append((grid_code.count("\n", 0, idx) + 1, needle, idx != -1))
columns = int("".join(c for c in grid_code.split("Grid::make(")[1].split(")")[0].strip() if c.isdigit() or c == "-"))
print("grid_columns:", columns)
print("component_count:", sum(1 for _, _, exists in components if exists))
print("components:")
for line, name, exists in components:
    print(f"  line {line}: {name} exists={exists}")
print("total_column_spans_nearby:", grid_code[:grid_code.index("                        ],")].count("columnSpan"))
print("total_column_spans_nearby_with_context:")
for needle in ["columnSpan"]:
    print(grid_code.count(needle))
PY

Repository: InvoicePlane/InvoicePlane-v2

Length of output: 5647

---

Keep invoice item fields on one row.

When show_line_item_position_numbers is enabled, Grid::make(6) receives seven one-column fields (position_number plus the invoice item fields), so subtotal moves to a second row. Increase the grid capacity to 7 for the item fields, or reduce subtotal’s column span.

🤖 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 `@Modules/Invoices/Filament/Company/Resources/Invoices/Schemas/InvoiceForm.php`
at line 165, Update the invoice item grid configuration around
getPositionNumberSchema() so enabling show_line_item_position_numbers keeps all
seven one-column fields, including subtotal, on a single row. Increase the Grid
capacity from 6 to 7, or equivalently reduce subtotal’s column span without
changing other item-field behavior.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Calculate the position from the repeater row identity.

product_id is not unique per invoice item. If two rows select the same product, both rows return the first matching position. New rows with product_id === null have the same failure.

Derive the position from the current repeater row key or state path. Add coverage for duplicate-product rows and reordered rows.

🤖 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 `@Modules/Invoices/Filament/Company/Resources/Invoices/Schemas/InvoiceForm.php`
around lines 316 - 331, Update the position calculation around the current
repeater row logic to identify the row by its repeater key or state path rather
than product_id. Ensure duplicate-product rows, null-product new rows, and
reordered rows each resolve to their own current position, and add coverage for
these cases.

[coderabbitai]

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

An unexpected error occurred while generating fixes: Not Found - https://docs.github.com/rest/git/refs#get-a-reference