Re-instated the older SMTP code

[jimleroyer]

Summary | Résumé

This is reinstating the older SMTP integration; for testing purposes only at the moment.

This might need newer integration code with SMTP mail manager that would simplify the AWS SMTP relay setup.

Related Issues | Cartes liées

Side of the table type of thing.

Test instructions | Instructions pour tester la modification

None at the moment.

Release Instructions | Instructions pour le déploiement

None.

Reviewer checklist | Liste de vérification du réviseur

• This PR does not break existing functionality.
• This PR does not violate GCNotify's privacy policies.
• This PR does not raise new security concerns. Refer to our GC Notify Risk Register document on our Google drive.
• This PR does not significantly alter performance.
• Additional required documentation resulting of these changes is covered (such as the README, setup instructions, a related ADR or the technical documentation).

⚠ If boxes cannot be checked off before merging the PR, they should be moved to the "Release Instructions" section with appropriate steps required to verify before release. For example, changes to celery code may require tests on staging to verify that performance has not been affected.

To resolve this issue, the keyword argument smtp_user should be replaced or removed so that only supported parameters are passed to create_service. This fix should be made on line 3364.

• First, review the parameters accepted by create_service (likely in tests/app/db.py), and determine which one is appropriate for setting an SMTP username. If smtp_user should be smtp_username or another name, update it accordingly.
• If there is no parameter for SMTP username, create the service without it, and set the attribute afterwards if necessary (e.g., via service.smtp_user = "foo").
• No import changes are needed.
• Edits occur only within the function call on line 3364 inside the method test_create_smtp_relay_for_service_if_it_already_has_one in tests/app/service/test_rest.py.

---

To fix this error, we need to ensure that calls to create_service() only include valid keyword arguments as defined by its signature. On line 3375, the argument smtp_user=None is passed, but CodeQL reports that it is not a valid parameter. The best fix is to remove the unsupported keyword argument (smtp_user=None) from the call to create_service(). This will prevent runtime errors without altering the intended test logic. Only edit line 3375 within the method test_create_smtp_relay_for_service in file tests/app/service/test_rest.py.

No additional imports, method definitions, or changes are required.

To fix the problem, we need to ensure that the arguments passed to create_service align with its definition. Since CodeQL indicates that smtp_user is not a valid parameter, the best solution is to replace smtp_user with the correct argument name used in create_service. Typically, this will either be a similarly named argument like smtp_user_name or smtp_username, or another appropriate keyword. The fix should be applied to every instance where create_service is called with the invalid keyword, so the following lines should be changed (as seen in the code above): 3364, 3375, 3404, 3431, 3444, and 3455. We need to identify or assume the correct parameter name (such as smtp_username) and update those calls accordingly. No new imports, methods, or major logic changes are needed—just the correction of the argument name in-place.

The best way to fix this issue is to use a supported parameter name in calls to create_service. Given the pattern of create_service(service_name=..., smtp_user=...), and the error indicating that smtp_user is unsupported, it's likely the intended supported parameter is named differently, possibly just smtp_username, smtp, or similar. If the correct way to specify the SMTP user for the service is to use a parameter such as smtp_username or smtp_user_name, then we should use that. Otherwise, if there is no way to set it in create_service, we should remove the keyword argument and set it after creation if required.

To make the minimal change, review all calls to create_service with the unsupported argument and rename smtp_user to the correct name, as per the expected function signature. If the correct parameter name is not given, or cannot be inferred, the argument should be removed and service.smtp_user set directly after service creation.

You need to update the following lines within class TestSmtpRelay in tests/app/service/test_rest.py:

• Line 3364: create_service(service_name="ABCDEF", smtp_user="foo")
• Line 3375: create_service(service_name="ABCDEF", smtp_user=None)
• Line 3404: create_service(service_name="ABCDEF", smtp_user="FOO-BAR")
• Line 3431: create_service(service_name="ABCDEF", smtp_user=None)
• Line 3444: create_service(service_name="ABCDEF", smtp_user=None)
• Line 3455: create_service(service_name="ABCDEF", smtp_user="foo")

Replace the incorrect keyword parameter with the correct one, or set the attribute after creation if needed.

---

To fix the issue, ensure the first parameter of the method test_create_smtp_relay_for_service is named self. You should reorder parameters so self is first, before mocker and any other fixtures. This change should be made directly to the method signature (line 3374), and the method's body does not require further updates since self is not referenced in the body. No additional imports, method definitions, or major changes are needed besides updating the parameter list.

To fix this issue without altering the functionality, the best approach is to decorate all methods in the TestSmtpRelay class with @staticmethod. Since none of these methods make use of self (the instance), and they're designed as independent pytest test functions, marking them as static methods is the recommended fix. Specifically, edit the relevant region of the file tests/app/service/test_rest.py, and add @staticmethod directly above each method defined in the TestSmtpRelay class. No changes to parameters or imports are required.

---

To fix this problem, you should replace the first parameter mocker with self, and shift all the parameters one place to the right. This change should be made in the definition line for test_get_smtp_relay_for_service_returns_empty_if_none. Additionally, you need to update all references to mocker within the body of this method to refer to the newly shifted parameter. The same reasoning applies if there are any other affected test methods in this class. No additional imports or method definitions are required.

Edit the specific function definition on or around line 3430 in tests/app/service/test_rest.py: change the function signature from

def test_get_smtp_relay_for_service_returns_empty_if_none(mocker, client, notify_db, notify_db_session):

to

def test_get_smtp_relay_for_service_returns_empty_if_none(self, mocker, client, notify_db, notify_db_session):

and update the body accordingly if mocker is used.

To fix this error, the function test_delete_smtp_relay_for_service_returns_500_if_none inside the TestSmtpRelay class should have its first parameter named self instead of mocker. The parameter list should begin with self, which will make the method an instance method as expected by Python's OO system and pytest's test collection when using class-based tests. Update the method definition so that self is the first parameter, and shift subsequent arguments accordingly.

Only edit the signature on the affected function definition (line 3443).

To fix the error, update the definition of the method test_delete_smtp_relay_for_service_returns_201_if_success in the TestSmtpRelay class so that the first parameter is named self instead of mocker. If you still want to use mocker (a pytest fixture), it should be added after self in the parameter list. Update all references to the parameters accordingly. Only the method signature needs to change; the method body likely does not require further modification unless there are references to the parameters by position.

Edit the method definition at line 3454 in the file tests/app/service/test_rest.py as follows:

• Change def test_delete_smtp_relay_for_service_returns_201_if_success(mocker, client, notify_db, notify_db_session):
  to
  def test_delete_smtp_relay_for_service_returns_201_if_success(self, mocker, client, notify_db, notify_db_session):

No additional imports or definitions are required.