Refactoring Immutabilité + PartnerInventory by fleuronvilik · Pull Request #4 · fleuronvilik/bookflow-core

fleuronvilik · 2026-03-31T07:37:44Z

Understand the issue: pi is None in void_sales_report silently returns early instead of raising an error
Create infra/errors.py with DataIntegrityError (with docstring)
Update app/use_cases.py void_sales_report to raise DataIntegrityError when inventory lines are missing, including partner_id and book_ids in the error message
All 122 tests pass
Code review addressed (error message specificity, docstring)
No CodeQL security alerts

…xt and use cases

…ansaction control

- add id to SalesReport, return a new instance on void, and reload persisted reports - align repository mapping, helpers, and tests with the immutable sales report model

- switch List, Dict, Set, Tuple, and Optional usages to built-in generic types - align domain, policies, use cases, and test helpers with the newer annotations

- return new inventory instances from deliver, sale, and restore operations - update use cases and add inventory behavior tests for sales and voids

- rename the delivery request repository method and update use case call sites - align test repo helpers and SQL repo tests with the new method name

Copilot

Pull request overview

This PR refactors core domain entities to be immutable and introduces a persisted PartnerInventory model, shifting “stock” from being projected from deliveries/reports to being stored and updated transactionally during delivery and sales report workflows.

Changes:

Make DeliveryRequest / SalesReport immutable (@dataclass(frozen=True)) and update transitions to return new instances.
Add partner_inventories table + SqlPartnerInventoryRepo, and wire it into Context/bootstrap/tests.
Update delivery + sales report use-cases to mutate partner inventory on mark_delivered, submit_sales_report, and void_sales_report.

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
tests/test_use_cases.py	Updates tests to new immutability/inventory-driven behavior.
tests/test_sql_sales_report_repo.py	Adapts repo tests to `SalesReport(id=...)` shape.
tests/test_sql_delivery_request_repo.py	Updates DR persistence tests to use `save(dr)` instead of `save_status`.
tests/test_sales_report.py	Updates SalesReport construction + removes `reports_by_partner` test.
tests/test_report_required_policy.py	Updates policy tests to use use-cases + helper fixtures.
tests/test_policies.py	Adjusts stock projection tests and adds a voided-sales case.
tests/test_partner_inventory.py	Adds new tests covering persisted inventory behavior.
tests/test_delivery_request.py	Updates tests for immutable transitions (return new DR).
tests/helpers.py	Refactors test helpers to use use-cases for DR/SR setup.
tests/conftest.py	Extends `Context` fixture with `pi_repo` and adjusts repo helpers.
runner/dispatch.py	Switches `stock` command to query persisted partner inventory.
policies/validations.py	Removes stock-validation logic (now handled via inventory).
policies/stock_projection.py	Typing cleanup (returns `dict[str, int]`).
policies/report_required.py	Minor typing cleanup.
infra/sql/sql_sales_report_repo.py	Adds optional `autocommit` and returns `SalesReport(id=...)` from `get`.
infra/sql/sql_partner_inventory_repo.py	New SQL repo implementing inventory upsert/get.
infra/sql/sql_delivery_request_repo.py	Refactors `save_status` into `save(dr)` and ensures DR `id` is populated on load.
infra/sql/sql_audit_repo.py	Adds optional `autocommit` for transactional audits.
infra/sql/schema.sql	Adds `partner_inventories` table.
domain/sales_report.py	Makes `SalesReport` immutable, adds `id`, and makes `void()` return a new instance.
domain/partner_inventory.py	New immutable domain entity for inventory changes.
domain/errors.py	Introduces `DomainError` base + adds `InsufficientStock`.
domain/delivery_request.py	Makes `DeliveryRequest` immutable, adds `id`, and returns new instances on transitions.
app/use_cases.py	Integrates inventory updates into delivered/report/void workflows with manual transactions.
app/queries.py	Adds `get_partner_inventory()` query.
app/context.py	Adds `pi_repo` to `Context`.
app/bootstrap.py	Wires `SqlPartnerInventoryRepo` into app context creation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

infra/sql/sql_partner_inventory_repo.py

infra/sql/schema.sql

tests/helpers.py

tests/test_partner_inventory.py

domain/partner_inventory.py

tests/test_policies.py

app/use_cases.py

- rename partner inventory sale methods to snake_case and update call sites - fix the inventory upsert conflict key, move InsufficientStock to the proper import, and add the missing SQL semicolon

- skip inventory restoration when no partner inventory row exists - clean up sales report test helpers and use the created report id in policy tests

Copilot

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

domain/delivery_request.py

app/use_cases.py

tests/test_sql_delivery_request_repo.py

tests/test_partner_inventory.py

tests/test_use_cases.py

tests/test_policies.py

…d_sales_report Agent-Logs-Url: https://github.com/fleuronvilik/bookflow-core/sessions/80754b8e-0943-4df9-97e7-c077437229e1 Co-authored-by: fleuronvilik <34385716+fleuronvilik@users.noreply.github.com>

- remove unused imports and dead commented test blocks - simplify delivery request and partner inventory tests for the current helpers

- replace the unused DataIntegrityError path with an inline note in void_sales_report - document why missing inventory lines are ignored for this project scope

Copilot

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-31T11:10:26Z

app/use_cases.py

+        for it in sr.items:
+            pi = ctx.pi_repo.get(sr.partner_id, it.book_id)
+            pi = pi.restore_sales(it.quantity)
+            ctx.pi_repo.save(pi, autocommit=False)
+


void_sales_report assumes a partner inventory row always exists (pi = ctx.pi_repo.get(...); pi = pi.restore_sales(...)). If pi is None, this will raise an AttributeError and the caller won’t get the intended domain/infrastructure error explaining the missing inventory lines. Add an explicit None check and raise DataIntegrityError (including partner_id + missing book_ids) before performing any DB writes, as described in the PR metadata/prior thread, so the failure mode is deterministic and actionable.

Copilot · 2026-03-31T11:10:26Z

app/use_cases.py

 from .context import Context
 from .helpers import get_dr_or_raise, get_sr_or_raise
 from .errors import ValidationError
+# from infra.errors import DataIntegrityError # leaving it there for now


There is a commented-out DataIntegrityError import left in the module. Given infra/errors.py is now part of the PR, either wire it into void_sales_report (and any other relevant integrity checks) or remove this comment to avoid dead/unclear code paths.

Suggested change

# from infra.errors import DataIntegrityError # leaving it there for now

app/use_cases.py

domain/sales_report.py

tests/test_sql_delivery_request_repo.py

- move sales report inventory restoration into restore_quantities_or_raise - raise a data integrity error when a report item has no matching inventory line

- cover voiding a sales report after deleting one inventory line - assert DataIntegrityError is raised with the expected missing book id

- fix grammar in delivery request and sales report error messages - rename restore_sales to restore_sale for consistency

Copilot

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-31T12:41:10Z

app/bootstrap.py

    return Context(
        catalog=catalog,
        dr_repo=SqlDeliveryRequestRepo(conn),
        sr_repo=SqlSalesReportRepo(conn),
+        pi_repo=SqlPartnerInventoryRepo(conn),
        audit=SqlAuditRepo(conn),
    )


make_ctx only initializes schema.sql when delivery_requests doesn't exist. Since Context now always includes pi_repo, running against an existing bookflow.db (already has delivery_requests) will not create the new partner_inventories table, leading to runtime failures when stock/inventory code runs. Consider checking for (and creating/migrating) partner_inventories as well, or introducing a simple schema migration step.

Copilot · 2026-03-31T12:41:10Z

app/helpers.py

+            missing_items_ids.append(it.book_id)
+            continue
+        pi = pi.restore_sale(it.quantity)
+        ctx.pi_repo.save(pi, autocommit=False)


restore_quantities_or_raise accepts an autocommit parameter but ignores it and always calls ctx.pi_repo.save(..., autocommit=False). This is misleading for callers and could cause subtle transaction bugs if the function is reused elsewhere. Either remove the parameter or thread it through consistently (and document the expected transaction boundary).

Suggested change

ctx.pi_repo.save(pi, autocommit=False)

ctx.pi_repo.save(pi, autocommit=autocommit)

Copilot · 2026-03-31T12:41:11Z

app/helpers.py

+    if missing_items_ids:
+        raise DataIntegrityError(
+            f"Sales report with id {sr.id} contains following items ({', '.join(missing_items_ids)}), for which no inventory line exists"
+        )


The raised DataIntegrityError message for missing inventory lines does not include partner_id, even though this error is partner-scoped and the PR description mentions including it. Including sr.partner_id in the message would make debugging much easier (especially if the same sr.id could exist across environments/log streams).

Copilot · 2026-03-31T12:41:11Z

tests/test_use_cases.py

-    dr.approve()
-    dr.mark_delivered()
-    ctx.dr_repo.create(dr)
+    # Stock entrant: 2 examplaires de b1 et 3 de b2 livrés au partenaire


Typo in comment: "examplaires" should be "exemplaires".

Suggested change

# Stock entrant: 2 examplaires de b1 et 3 de b2 livrés au partenaire

# Stock entrant: 2 exemplaires de b1 et 3 de b2 livrés au partenaire

fleuronvilik added 9 commits March 19, 2026 15:08

feat: implement partner inventory management and integrate into conte…

56b6eed

…xt and use cases

feat: enhance repository methods with autocommit option for better tr…

fb35737

…ansaction control

refactor(delivery-request): make entity immutable and update DR workflow

6bcbf05

refactor(domain): centralize domain exceptions

c3195bb

test: align helpers and policy scenarios with immutable flow

c363e36

refactor(sales-report): make entity immutable

f9b03b8

- add id to SalesReport, return a new instance on void, and reload persisted reports - align repository mapping, helpers, and tests with the immutable sales report model

refactor(typing): replace legacy collection aliases

05e7a4f

- switch List, Dict, Set, Tuple, and Optional usages to built-in generic types - align domain, policies, use cases, and test helpers with the newer annotations

refactor(partner-inventory): make entity immutable

11c7c18

- return new inventory instances from deliver, sale, and restore operations - update use cases and add inventory behavior tests for sales and voids

refactor(delivery-request): rename save_status to save

5272013

- rename the delivery request repository method and update use case call sites - align test repo helpers and SQL repo tests with the new method name

fleuronvilik requested a review from Copilot March 31, 2026 07:37

Copilot started reviewing on behalf of fleuronvilik March 31, 2026 07:38 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

fleuronvilik marked this pull request as draft March 31, 2026 07:56

fleuronvilik added 2 commits March 31, 2026 09:58

fix(partner-inventory): align naming and imports

fddd3c5

- rename partner inventory sale methods to snake_case and update call sites - fix the inventory upsert conflict key, move InsufficientStock to the proper import, and add the missing SQL semicolon

fix(void): handle missing inventory on sales report void

30a8fd8

- skip inventory restoration when no partner inventory row exists - clean up sales report test helpers and use the created report id in policy tests

fleuronvilik marked this pull request as ready for review March 31, 2026 08:17

fleuronvilik requested a review from Copilot March 31, 2026 08:18

Copilot started reviewing on behalf of fleuronvilik March 31, 2026 08:18 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

Copilot started work on behalf of fleuronvilik March 31, 2026 09:14 View session

fix(void): raise DataIntegrityError on missing inventory lines in voi…

764ad30

…d_sales_report Agent-Logs-Url: https://github.com/fleuronvilik/bookflow-core/sessions/80754b8e-0943-4df9-97e7-c077437229e1 Co-authored-by: fleuronvilik <34385716+fleuronvilik@users.noreply.github.com>

Copilot finished work on behalf of fleuronvilik March 31, 2026 09:19

fleuronvilik added 3 commits March 31, 2026 12:08

chore(tests): clean up outdated test code and comments

f1daea3

- remove unused imports and dead commented test blocks - simplify delivery request and partner inventory tests for the current helpers

fix(delivery-request): comment out unused __str__

756d493

docs(void): explain skipped data integrity handling

0b6d38a

- replace the unused DataIntegrityError path with an inline note in void_sales_report - document why missing inventory lines are ignored for this project scope

fleuronvilik requested a review from Copilot March 31, 2026 11:05

Copilot started reviewing on behalf of fleuronvilik March 31, 2026 11:05 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

fleuronvilik added 2 commits March 31, 2026 13:34

fix(tests): save updated delivery request status in repo test

2464db2

feat(void): extract inventory restore helper

a5c56a7

- move sales report inventory restoration into restore_quantities_or_raise - raise a data integrity error when a report item has no matching inventory line

fleuronvilik added 2 commits March 31, 2026 14:28

tests(void): add data integrity error case for missing inventory

6a0e8b6

- cover voiding a sales report after deleting one inventory line - assert DataIntegrityError is raised with the expected missing book id

fix: correct typos and align restore method naming

1533001

- fix grammar in delivery request and sales report error messages - rename restore_sales to restore_sale for consistency

fleuronvilik requested a review from Copilot March 31, 2026 12:34

Copilot started reviewing on behalf of fleuronvilik March 31, 2026 12:35 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

fleuronvilik linked an issue Mar 31, 2026 that may be closed by this pull request

Refactoriser la persistance côté repository pour persister l'agrégat complet #2

Open

	ctx.pi_repo.save(pi, autocommit=False)
	ctx.pi_repo.save(pi, autocommit=autocommit)

	# Stock entrant: 2 examplaires de b1 et 3 de b2 livrés au partenaire
	# Stock entrant: 2 exemplaires de b1 et 3 de b2 livrés au partenaire

Conversation

fleuronvilik commented Mar 31, 2026 • edited by Copilot AI Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fleuronvilik commented Mar 31, 2026 •

edited by Copilot AI

Loading