DeathGun44 opened a new pull request, #6246:
URL: https://github.com/apache/fineract/pull/6246
## Description
Migrates five organisation and reference-data integration test classes from
REST-assured onto the generated Feign client.
| Class | Tests |
|---|---|
| `OfficeIntegrationTest` | 3 |
| `HookIntegrationTest` | 2 (1 disabled, see gaps) |
| `ExternalEventConfigurationIntegrationTest` | 2 |
| `RatesTest` | 1 |
| `ProductMixIntegrationTest` | 1 |
**No production code is changed.** Every file is under `integration-tests/`,
so the OpenAPI spec is untouched and `checkBreakingChanges` cannot regress. The
PR shares no file with any other in-flight migration branch and can be merged
in any order relative to them.
### Helpers
- `HookHelper` and `RatesHelper` are deleted; `ProductMixHelper` moves to
`client/feign/helpers/FeignProductMixHelper`. None had a caller outside its own
test.
- `ExternalEventConfigurationHelper` loses 788 lines - a hand-maintained
catalogue of the 146 default event configurations plus two helpers, all three
with no callers left. Its two live methods stay; three other tests still use
them.
- `OfficeHelper` is untouched: 20 other tests still depend on it.
- `FeignExternalEventHelper` now delegates its two configuration toggles to
`FeignExternalEventConfigurationHelper`, so one class owns that endpoint.
### Notable changes beyond the transport swap
- The 146 expected defaults collapse from 788 lines of maps to a set of type
names in `ExternalEventConfigurationTestData`. Keeping the catalogue duplicated
is deliberate: adding or removing an event type should fail this test until the
set is updated. (The previous list had drifted out of date against the
Liquibase seed.)
- The external-event update test re-reads the configuration afterwards,
rather than trusting only the echoed `changes` map.
- `RatesTest` additionally asserts the new rate appears in the listing, and
checks the update's `changes` payload against both the requested percentage and
the persisted one.
- `FeignHookHelper` locates the payload URL by field name instead of by list
position - the server promises no order there, so `config.get(1)` happened to
work rather than being correct. Field names come from the server's own
`HookApiConstants`, so a rename breaks the build instead of the test.
- `HookIntegrationTest` deletes its hook in `@AfterEach`. Previously a
failed assertion could leave an active `OFFICE`/`CREATE` webhook registered,
making every later test that creates an office fire an outbound dispatch.
### Known gaps left in place, each needing its own ticket
Pre-existing, called out rather than papered over:
1. **`CashierSummaryAndTransactionsTest` is not migrated.** It was in scope
until the typed client turned out to be unable to create a teller.
`TellerApiResourceSwagger.PostTellersRequest.status` is declared as
`TellerStatus`, so the generated model sends `"ACTIVE"`, but `Teller.fromJson`
reads the field with `integerValueOfParameterNamed("status")` and expects an
int. The server answers `400 error.msg.invalid.json.data`. Correcting the DTO
is production code and a breaking OpenAPI change, so it does not belong here.
2. **`ProductMixIntegrationTest` can only assert that its two calls returned
200.** The listing call does not send `associations=productMixes`, so it
returns loan productsrather than product mixes; and with
`isProductMixTemplate=true` the endpoint answers `{"productOptions": [...]}`,
which the declared `GetLoanProductsTemplateResponse` has no field for, leaving
every property null. Both are unchanged from before the migration and both need
`LoanProductsApiResourceSwagger` changes to close.
3. **`shouldSendOfficeCreationNotification` is `@Disabled`.** It polls
`echo-webhook.herokuapp.com`, offline since Heroku retired free dynos in 2022,
so the loop always timed out and the test passed having asserted nothing -
while still registering a hook and making the server attempt an outbound POST
to a dead host on every CI run. The code is kept for whoever supplies a
replacement echo endpoint.
## Checklist
Please make sure these boxes are checked before submitting your pull request
- thanks!
- [ ] Write the commit message as per [our
guidelines](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#pull-requests)
- [ ] Acknowledge that we will not review PRs that are not passing the build
_("green")_ - it is your responsibility to get a proposed PR to pass the build,
not primarily the project's maintainers.
- [ ] Create/update [unit or integration
tests](https://fineract.apache.org/docs/current/#_testing) for verifying the
changes made.
- [ ] Follow our [coding
conventions](https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions).
- [ ] Add required Swagger annotation and update API documentation at
fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with
details of any API changes
- [ ] [This PR must not be a "code
dump"](https://cwiki.apache.org/confluence/display/FINERACT/Pull+Request+Size+Limit).
Large changes can be made in a branch, with assistance. Ask for help on the
[developer mailing list](https://fineract.apache.org/#contribute).
- [ ] If merging this PR resolves a JIRA issue, I will mark that issue as
resolved and set "Fix Version/s" appropriately.
Your assigned reviewer(s) will follow our [guidelines for code
reviews](https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
