Test fix - assertion fails when run with other tests (sometimes)

[eileenmcnaughton]

Overview

Test fix - assertion fails when run with other tests (sometimes)

Before

Test only passes when the next autoincrement for civicrm_contribution.id is 1

After

Test passes regardless

Technical Details

The assertion that invoice_number = INV_ + contribution_id is wrong. In fact
it is calculated as INV_ + (MAX(contribution_id)+1) prior to the contribution
being saved. This means that if the table has not been truncated the contribution
id might be 3 but the invoice_number would still be 0+1 - ie INV_1

• we could argue about whether the calculation method is flawed but this at least
  makes the test more reliable and no-one has complained in practice about the
  code behaviour

Comments

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[demeritcowboy]

• General standards
  • [r-explain] Soft Pass
    • It would have been a quicker review but I had to hunt for it:

      civicrm-core/CRM/Contribute/BAO/Contribution.php

      Line 122 in 20a0376

      $nextContributionID = CRM_Core_DAO::singleValueQuery("SELECT COALESCE(MAX(id) + 1, 1) FROM civicrm_contribution");

    • It took me a bit to think through the scenario - "not truncated" but ALSO all the records have been deleted (via DELETE as opposed to truncate), so then the max calculation be null, but the autoincrement counter would still be at whatever it was before the DELETE for when the contribution is inserted.
    • The other changes are fixing the order of arguments to assert(), which were backwards.
  • [r-user] PASS
  • [r-doc] PASS
  • [r-run] PASS
• Developer standards
  • [r-tech] PASS
    • I suppose it's ok to do the invoice assert as you've done with the regex to leave it more open rather than enforce the possibly weird calculation.
  • [r-code] PASS
  • [r-maint] PASS
  • [r-test] PASS