Skip to content

SUBMIT-781 Create backend unit tests for models and updated invitation logic#831

Open
leodube-aot wants to merge 11 commits intobcgov:developfrom
leodube-aot:SUBMIT-781
Open

SUBMIT-781 Create backend unit tests for models and updated invitation logic#831
leodube-aot wants to merge 11 commits intobcgov:developfrom
leodube-aot:SUBMIT-781

Conversation

@leodube-aot
Copy link
Copy Markdown
Collaborator

@leodube-aot leodube-aot commented Mar 24, 2026

Ticket: SUBMIT-781 Create backend unit tests for work/phase/invitation/account management

Description

  • Created unit tests for all the model files (that have ORM operations).
  • Updated existing unit tests that needed updating (specifically regarding the latest changes around invitations, work+phase, and account management).
  • Did some minor refactoring to minimize deprecation warnings, and to keep ORM operations out of the services.

Refactoring

  • Fixed a bunch of deprecation warnings. The backend tests used to throw 228 warnings. Now they only throw 2 warnings. Most of these were fixed by updating datetime.utcnow to datetime.now(UTC).
  • Moved some methods out of /services into specific /models files. This was to try to keep direct orm operations in the models (ie. db.query operations)
  • Create a persist method on the BaseModel to avoid duplicate code.

cls._add_completed_status(aggregated_statuses, statuses)
cls._add_submitted_status(aggregated_statuses, statuses)
cls._add_passed_consultation_check(aggregated_statuses, statuses)
cls._add_review_rejected(aggregated_statuses, statuses)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicated from line 122

else:
account.save()
return account
return account.persist(session)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use this pattern quite frequently: if session - flush, else - save. I moved this into a common method called persist that does the same thing, but avoids duplicate code.

type_id = fields.Int(data_key="type_id")
status = fields.List(fields.Enum(enum=PackageStatus),
enum=PackageStatus, data_key="status")
status = fields.List(fields.Enum(enum=PackageStatus), data_key="status", metadata={"enum": PackageStatus})
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marshmallow 3.x deprecated passing extra metadata (like enum, description, example, etc.) as direct keyword arguments to fields. They need to be wrapped in an explicit metadata={} dict.

Comment on lines -117 to +110
user = UserModel.query.filter(UserModel.id == user_id).first()
return user.user_status.status_name if user else None
return UserModel.get_status_name_by_id(user_id)
Copy link
Copy Markdown
Collaborator Author

@leodube-aot leodube-aot Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part of the refactoring: Encapsulated Model.query.filter calls and moved into the model file itself. Now we call the encapsulated query. No logic actually changes here.

return query.order_by(cls.name).all()

@classmethod
def get_proponent_by_id(
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wasn't deleted, just moved into the proponent_service file. This method does a lot of orchestration work, pulling from several other db models. Makes more sense as a service.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant