[Fixes4BC (neeleshsinghal)]

Fixes AB#656806

[alexei-dobriansky]

Good Sense Reviewer - Round 1

Recommendation: Request Changes

What this PR does

The PR combines Address and Address 2 into the 70-character recipient address for customer, vendor, employee, and bank recipients in CH. Both Swiss SEPA XML formats use this field for the creditor street name.

The shared helper handles one or both address fields being empty and preserves the existing recipient events. However, the employee setter calls an undefined procedure, so the CH BaseApp cannot compile. The new tests generate empty address values, so they do not prove that building numbers are exported.

Problem-solution fit

Fit: Partial

Combining the two address fields addresses the missing building number at the shared export-buffer level. The compilation error prevents the fix from being used, and the new fixtures do not test a populated street and building number.

Suggestions

S1 (🔴 High): Call the existing helper for employee addresses
Replace RecipientAddress with SetRecipientAddress. RecipientAddress does not exist, so the CH application cannot compile.

S2 (🟠 Moderate): Use nonempty addresses in the version 09 tests
Use nonempty street and building-number values whose combined length fits 70 characters. Both variables start empty, so StrLen returns zero and RandText leaves them empty. Give the blank-Address-2 companion test a nonempty street too.

S3 (🟠 Moderate): Use nonempty addresses in the version 03 tests
Use nonempty street and building-number values whose combined length fits 70 characters. Both variables start empty, so StrLen returns zero and RandText leaves them empty. Give the blank-Address-2 companion test a nonempty street too.

Risk assessment and necessity

Risk: All four recipient types and payment exports that use the CH table are affected, including non-Swiss formats within CH. Public signatures, field definitions, and event timing are unchanged; there is no schema upgrade or amount-calculation change. The current employee call blocks compilation.

Necessity: The change is needed to avoid dropping building numbers from exported creditor addresses when Address 2 holds them. Without a fix, users must correct exported XML or combine address data manually, which conflicts with keeping street and building number separate. A shared CH helper is a small, targeted solution.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=13273 round=1 by=alexei-dobriansky at=2026-10-26T20:39:44.5663190+00:00 lastSha=f2724dcccc54436dbfdfc77750d8abfa02c2a44c reviewKey=774b730ac5938c4c6e3c503c4bc6a58b2f738d0d60e08cb78e96150243c2b39d suggestions=S1@1e14b313,S2@0d86ac1d,S3@ff366734

[alexei-dobriansky]

Good Sense Reviewer - Round 2

Recommendation: Accept

What this PR does

The PR combines Address and Address 2 into the 70-character recipient address used by CH payment exports. The latest commits correct the employee helper call and make both SEPA test suites use real street and building-number values.

The helper now handles either address field being blank, preserves the existing recipient events, and keeps the field-length limit. The compile blocker and both ineffective tests from round 1 are fixed.

Status of previous suggestions

ID	Title	Status	Author response
S1	Call the existing helper for employee addresses	Addressed	Fixed in b4ed807.
S2	Use nonempty addresses in the version 09 tests	Addressed	Fixed in b02de9a.
S3	Use nonempty addresses in the version 03 tests	Addressed	Fixed in b02de9a.

New observations (commits since round 1)

None - changes only addressed prior suggestions.

Risk assessment and necessity

Risk: The shared CH payment export table feeds customer, vendor, employee, and bank recipient data. The helper keeps the 70-character limit and existing recipient event timing, so the remaining regression surface is limited to address formatting.

Necessity: The change is needed so Swiss payment exports keep the building number when it is stored in Address 2. The shared helper is a focused fix, and the two SEPA test suites now exercise populated and blank Address 2 values.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=13273 round=2 by=alexei-dobriansky at=2026-10-27T14:10:27.6524621+00:00 lastSha=b02de9ab281b69ab47fb608fb5991274c58d8e5a reviewKey=c3304f49a7eaa44fc463104e1107afcd07a47279e1fe9c318228369c15edab2c suggestions=S1@1e14b313:addressed,S2@0d86ac1d:addressed,S3@ff366734:addressed parentRound=1

[alexei-dobriansky]

Good Sense Reviewer - Round 3

Recommendation: Accept with Suggestions

What this PR does

The PR combines Address and Address 2 into the 70-character recipient address so Swiss credit transfers retain a separately stored building number. The latest commits keep the original address-only behavior when Swiss credit-transfer mode is disabled.

The credit-transfer filler sets the mode before selecting the recipient setter. In Swiss mode, the helper joins populated fields with one space and keeps the length limit; otherwise, it returns the first 70 characters of Address. Both credit-transfer XML formats use that result directly for the creditor street. The earlier helper-call and test-fixture problems remain fixed.

Problem-solution fit

Fit: Strong

The reported Swiss credit-transfer case now retains the street and building number, while other export paths keep their original address-only result. Direct-debit formatting remains unchanged rather than gaining the same concatenation.

Status of previous suggestions

ID	Title	Status	Author response
S1	Call the existing helper for employee addresses	Addressed	The employee setter still calls the defined SetRecipientAddress helper.
S2	Use nonempty addresses in the version 09 tests	Addressed	Both tests still use positive fixture lengths and compare exact recipient addresses.
S3	Use nonempty addresses in the version 03 tests	Addressed	Both tests still use positive fixture lengths and compare exact recipient addresses.

New observations (commits since round 2)

S4 (🟠 Moderate): Cover the new non-Swiss address guard
Consider adding a test with SwissExport set to false and a nonempty Address 2. Assert that Recipient Address contains only the original street, so the new guard remains covered.

Risk assessment and necessity

Risk: The shared helper changes recipient address formatting for customer, vendor, employee, and bank credit transfers in CH. The current non-Swiss branch preserves the original primary address; direct-debit formatting also stays unchanged. The four added tests check real Swiss vendor export buffers for populated and blank Address 2, but do not cover the new false branch. No amounts, posting logic, table schema, public signatures, or event timing change. This assessment is based on source analysis; AL builds and tests were not run, and the captured CI checks were still in progress.

Necessity: Retaining a building number stored in Address 2 is needed for complete Swiss creditor addresses. Without the correction, users must combine the street and building number manually in Address where possible, which conflicts with keeping them separate for QR bills. The fix is focused and keeps other exports' established address behavior.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=13273 round=3 by=alexei-dobriansky at=2026-10-28T08:52:08.6872944Z lastSha=8c3e438dce09da3ef4cf37cf42e9d37d6cba889d reviewKey=d973f876981cd9908c60544f9a6e2c17f28100e92042ffe17de9d9d4779217e9 suggestions=S1@1e14b313:addressed,S2@0d86ac1d:addressed,S3@ff366734:addressed,S4@3a4d267a:new parentRound=2