fix(contacts): make update_company and update_contact persist every Update*Request field #74

Merged
YousifShkara merged 1 commit from feat/pms-17-tests into main 2026-06-05 03:30:05 +02:00
Owner

The integration tests added in the previous commit exposed two real
defects, same family as the F4 update_site bug that PMS-18 fixed:

update_contact (service.rs) only handled first_name, last_name, email.
Writes to title, department, phone, mobile, fax, contact_type,
preferred_contact_method, timezone, custom_fields, tags, notes,
status, and company_id were silently dropped on a 200 OK.

update_company (service.rs) only handled name, company_type, status.
Writes to industry, website, phone, fax, address, billing_address,
tax_id, account_number, default_billing_contact_id,
default_technical_contact_id, account_manager_id, sla_id,
default_contract_id, payment_terms, tax_exempt, custom_fields, tags,
notes, portal_enabled, and parent_company_id were silently dropped on
a 200 OK. The old code even contained a 'Add more fields as needed...'
placeholder comment.

Both methods rewritten to use the same dynamic-UPDATE pattern as
update_site: collect SET-fragment strings, build the SQL once,
chain .bind() calls in the same order. Every Update*Request field
now round-trips. validate_fk on company_id is kept on the contact
path (matches the create path) so a contact can't be moved into
another tenant's company.

PreferredContactMethod has no as_str(); the contact-update binds it
via a local match against the table's CHECK constraint ('email',
'phone', 'mobile').

Tests:

  • Existing site_crud_happy_path / company list-page sub-resource
    tests learned that GET /contacts/companies/{id}/sites and
    /contacts/contacts return PaginatedResponse envelopes ({data,
    meta}) and now read list["data"] instead of treating the
    response as a bare array. Three call sites in site_crud_happy_path.
  • contact_crud_happy_path extended to PUT and re-GET title,
    department, phone, AND mobile so the regression covers more than
    just two scalars.
  • New company_update_persists_all_fields test: F4-bis pin for
    companies. PUT name + industry + website + phone + address
    (nested) + payment_terms + tax_exempt + notes, then independent
    GET and assert every value landed.

Verified: cargo check --tests, cargo clippy --tests -- -Dwarnings,
cargo fmt --all --check all clean, and cargo test --test contacts
runs 7/7 tests green against a live postgres.

The integration tests added in the previous commit exposed two real defects, same family as the F4 update_site bug that PMS-18 fixed: update_contact (service.rs) only handled first_name, last_name, email. Writes to title, department, phone, mobile, fax, contact_type, preferred_contact_method, timezone, custom_fields, tags, notes, status, and company_id were silently dropped on a 200 OK. update_company (service.rs) only handled name, company_type, status. Writes to industry, website, phone, fax, address, billing_address, tax_id, account_number, default_billing_contact_id, default_technical_contact_id, account_manager_id, sla_id, default_contract_id, payment_terms, tax_exempt, custom_fields, tags, notes, portal_enabled, and parent_company_id were silently dropped on a 200 OK. The old code even contained a 'Add more fields as needed...' placeholder comment. Both methods rewritten to use the same dynamic-UPDATE pattern as update_site: collect SET-fragment strings, build the SQL once, chain .bind() calls in the same order. Every Update*Request field now round-trips. validate_fk on company_id is kept on the contact path (matches the create path) so a contact can't be moved into another tenant's company. PreferredContactMethod has no as_str(); the contact-update binds it via a local match against the table's CHECK constraint ('email', 'phone', 'mobile'). Tests: - Existing site_crud_happy_path / company list-page sub-resource tests learned that GET /contacts/companies/{id}/sites and /contacts/contacts return PaginatedResponse envelopes ({data, meta}) and now read list["data"] instead of treating the response as a bare array. Three call sites in site_crud_happy_path. - contact_crud_happy_path extended to PUT and re-GET title, department, phone, AND mobile so the regression covers more than just two scalars. - New company_update_persists_all_fields test: F4-bis pin for companies. PUT name + industry + website + phone + address (nested) + payment_terms + tax_exempt + notes, then independent GET and assert every value landed. Verified: cargo check --tests, cargo clippy --tests -- -Dwarnings, cargo fmt --all --check all clean, and cargo test --test contacts runs 7/7 tests green against a live postgres.
fix(contacts): make update_company and update_contact persist every Update*Request field
All checks were successful
Create release / Create release from merged PR (pull_request) Has been skipped
Check / fmt + clippy + compile + tests (pull_request) Successful in 6m54s
Build OCI container / Build and push mokosh-api image (push) Successful in 9m5s
3bbf1e548e
The integration tests added in the previous commit exposed two real
defects, same family as the F4 update_site bug that PMS-18 fixed:

update_contact (service.rs) only handled first_name, last_name, email.
Writes to title, department, phone, mobile, fax, contact_type,
preferred_contact_method, timezone, custom_fields, tags, notes,
status, and company_id were silently dropped on a 200 OK.

update_company (service.rs) only handled name, company_type, status.
Writes to industry, website, phone, fax, address, billing_address,
tax_id, account_number, default_billing_contact_id,
default_technical_contact_id, account_manager_id, sla_id,
default_contract_id, payment_terms, tax_exempt, custom_fields, tags,
notes, portal_enabled, and parent_company_id were silently dropped on
a 200 OK. The old code even contained a 'Add more fields as needed...'
placeholder comment.

Both methods rewritten to use the same dynamic-UPDATE pattern as
update_site: collect SET-fragment strings, build the SQL once,
chain .bind() calls in the same order. Every Update*Request field
now round-trips. validate_fk on company_id is kept on the contact
path (matches the create path) so a contact can't be moved into
another tenant's company.

PreferredContactMethod has no as_str(); the contact-update binds it
via a local match against the table's CHECK constraint ('email',
'phone', 'mobile').

Tests:
- Existing site_crud_happy_path / company list-page sub-resource
  tests learned that GET /contacts/companies/{id}/sites and
  /contacts/contacts return PaginatedResponse envelopes ({data,
  meta}) and now read list["data"] instead of treating the
  response as a bare array. Three call sites in site_crud_happy_path.
- contact_crud_happy_path extended to PUT and re-GET title,
  department, phone, AND mobile so the regression covers more than
  just two scalars.
- New company_update_persists_all_fields test: F4-bis pin for
  companies. PUT name + industry + website + phone + address
  (nested) + payment_terms + tax_exempt + notes, then independent
  GET and assert every value landed.

Verified: cargo check --tests, cargo clippy --tests -- -Dwarnings,
cargo fmt --all --check all clean, and cargo test --test contacts
runs 7/7 tests green against a live postgres.
YousifShkara deleted branch feat/pms-17-tests 2026-06-05 03:30:05 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
psa-systems/mokosh-server!74
No description provided.