-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Issue #3536] Add saved opportunity notifications to backend job #3639
Merged
mikehgrantsgov
merged 43 commits into
main
from
mikehgrantsgov/3536-add-saved-notifications-to-job
Jan 29, 2025
Merged
Changes from all commits
Commits
Show all changes
43 commits
Select commit
Hold shift + click to select a range
ae394d2
Add JobTable, track in tasks
mikehgrantsgov ea3d22f
Create ERD diagram and Update OpenAPI spec
nava-platform-bot 98257e3
Update to enums / add metrics to table
mikehgrantsgov 8fa48f5
Merge branch 'mikehgrantsgov/3527-modify-load-opp-logic-never-delete'…
mikehgrantsgov 71a80d9
Lint
mikehgrantsgov e65f8d7
Lint
mikehgrantsgov 53d7b2e
Merge branch 'main' into mikehgrantsgov/3527-modify-load-opp-logic-ne…
mikehgrantsgov ac89d27
Create ERD diagram and Update OpenAPI spec
nava-platform-bot 4193c2b
Update api/src/task/task.py
mikehgrantsgov 4f72d43
Update api/src/task/task.py
mikehgrantsgov 238e8fd
Update api/src/task/task.py
mikehgrantsgov 43f90be
Remove last_loaded_at and use updated_at instead
mikehgrantsgov 5c18c81
Merge branch 'mikehgrantsgov/3527-modify-load-opp-logic-never-delete'…
mikehgrantsgov 2865fbd
Create ERD diagram and Update OpenAPI spec
nava-platform-bot b5c8d05
Lint
mikehgrantsgov 68030d4
Merge branch 'mikehgrantsgov/3527-modify-load-opp-logic-never-delete'…
mikehgrantsgov 54b5d10
Fix
mikehgrantsgov 55922b8
Merge branch 'main' into mikehgrantsgov/3527-modify-load-opp-logic-ne…
mikehgrantsgov 92b21fb
Update migration
mikehgrantsgov 4fe3095
Fix migration
mikehgrantsgov 9efeb56
Update to JobLog, remove has_update
mikehgrantsgov 578fa09
Format
mikehgrantsgov e62f4e6
Remove query on non-null column
mikehgrantsgov ad38093
Add task tests
mikehgrantsgov 562c2a7
Catch db errors and rollback/start new transaction to store failed state
mikehgrantsgov 624bd98
Merge branch 'main' into mikehgrantsgov/3527-modify-load-opp-logic-ne…
mikehgrantsgov e57fc43
Fix head
mikehgrantsgov aa65d75
Create ERD diagram and Update OpenAPI spec
nava-platform-bot 4342630
Fix migration
mikehgrantsgov 9d51544
Merge branch 'mikehgrantsgov/3527-modify-load-opp-logic-never-delete'…
mikehgrantsgov aa6f5b7
Update transaction management
mikehgrantsgov 2761cdf
Fix test
mikehgrantsgov 6c26a5d
Wrap failed update with db_session.begin
mikehgrantsgov 35817e7
Detect and notify when an opportunity is changed
mikehgrantsgov e9ad242
Create ERD diagram and Update OpenAPI spec
nava-platform-bot f20a8a1
Merge branch 'main' into mikehgrantsgov/3536-add-saved-notifications-…
mikehgrantsgov ac20a08
Add new tests / PR feedback
mikehgrantsgov 2b089a9
Merge branch 'main' into mikehgrantsgov/3536-add-saved-notifications-…
mikehgrantsgov aff14dd
Create ERD diagram and Update OpenAPI spec
nava-platform-bot 7e1fd70
Change to enum
mikehgrantsgov 269a2a4
Merge branch 'main' into mikehgrantsgov/3536-add-saved-notifications-…
mikehgrantsgov 3b95ed6
Fix revision
mikehgrantsgov 1807ad9
Create ERD diagram and Update OpenAPI spec
nava-platform-bot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
34 changes: 34 additions & 0 deletions
34
api/src/db/migrations/versions/2025_01_24_add_last_notified_at_to_user_saved_.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
"""Add last_notified_at to user saved opportunity table | ||
|
||
Revision ID: 43b179a7c92e | ||
Revises: dc04ce955a9a | ||
Create Date: 2025-01-24 17:15:14.064880 | ||
|
||
""" | ||
|
||
import sqlalchemy as sa | ||
from alembic import op | ||
|
||
# revision identifiers, used by Alembic. | ||
revision = "43b179a7c92e" | ||
down_revision = "9e7fc937646a" | ||
branch_labels = None | ||
depends_on = None | ||
|
||
|
||
def upgrade(): | ||
# ### commands auto generated by Alembic - please adjust! ### | ||
op.add_column( | ||
"user_saved_opportunity", | ||
sa.Column( | ||
"last_notified_at", sa.TIMESTAMP(timezone=True), server_default="NOW()", nullable=False | ||
), | ||
schema="api", | ||
) | ||
# ### end Alembic commands ### | ||
|
||
|
||
def downgrade(): | ||
# ### commands auto generated by Alembic - please adjust! ### | ||
op.drop_column("user_saved_opportunity", "last_notified_at", schema="api") | ||
# ### end Alembic commands ### |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
127 changes: 127 additions & 0 deletions
127
api/tests/src/task/notifications/test_generate_notifications.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,132 @@ | ||
from datetime import timedelta | ||
|
||
import pytest | ||
|
||
import tests.src.db.models.factories as factories | ||
from src.db.models.user_models import UserNotificationLog | ||
from src.task.notifications.generate_notifications import NotificationConstants | ||
|
||
|
||
@pytest.fixture | ||
def clear_notification_logs(db_session): | ||
"""Clear all notification logs""" | ||
db_session.query(UserNotificationLog).delete() | ||
|
||
|
||
def test_via_cli(cli_runner, db_session, enable_factory_create, user): | ||
"""Simple test that verifies we can invoke the notification task via CLI""" | ||
result = cli_runner.invoke(args=["task", "generate-notifications"]) | ||
|
||
assert result.exit_code == 0 | ||
|
||
|
||
def test_collect_notifications_cli(cli_runner, db_session, enable_factory_create, user, caplog): | ||
"""Simple test that verifies we can invoke the notification task via CLI""" | ||
# Create a saved opportunity that needs notification | ||
opportunity = factories.OpportunityFactory.create() | ||
saved_opportunity = factories.UserSavedOpportunityFactory.create( | ||
user=user, | ||
opportunity=opportunity, | ||
last_notified_at=opportunity.updated_at - timedelta(days=1), | ||
) | ||
factories.OpportunityChangeAuditFactory.create( | ||
opportunity=opportunity, | ||
updated_at=saved_opportunity.last_notified_at + timedelta(minutes=1), | ||
) | ||
|
||
result = cli_runner.invoke(args=["task", "generate-notifications"]) | ||
|
||
assert result.exit_code == 0 | ||
|
||
# Verify expected log messages | ||
assert "Collected opportunity notifications" in caplog.text | ||
assert "Would send notification to user" in caplog.text | ||
|
||
# Verify the log contains the correct metrics | ||
log_records = [r for r in caplog.records if "Would send notification to user" in r.message] | ||
assert len(log_records) == 1 | ||
extra = log_records[0].__dict__ | ||
assert extra["user_id"] == user.user_id | ||
assert extra["opportunity_count"] == 1 | ||
assert extra["search_count"] == 0 | ||
|
||
|
||
def test_last_notified_at_updates(cli_runner, db_session, enable_factory_create, user): | ||
"""Test that last_notified_at gets updated after sending notifications""" | ||
# Create an opportunity that was updated after the last notification | ||
opportunity = factories.OpportunityFactory.create() | ||
saved_opp = factories.UserSavedOpportunityFactory.create( | ||
user=user, | ||
opportunity=opportunity, | ||
last_notified_at=opportunity.updated_at - timedelta(days=1), | ||
) | ||
factories.OpportunityChangeAuditFactory.create( | ||
opportunity=opportunity, | ||
updated_at=saved_opp.last_notified_at + timedelta(minutes=1), | ||
) | ||
# Store the original notification time | ||
original_notification_time = saved_opp.last_notified_at | ||
|
||
# Run the notification task | ||
result = cli_runner.invoke(args=["task", "generate-notifications"]) | ||
assert result.exit_code == 0 | ||
|
||
# Refresh the saved opportunity from the database | ||
db_session.refresh(saved_opp) | ||
|
||
# Verify last_notified_at was updated | ||
assert saved_opp.last_notified_at > original_notification_time | ||
# Verify last_notified_at is now after the opportunity's updated_at | ||
assert saved_opp.last_notified_at > opportunity.updated_at | ||
|
||
|
||
def test_notification_log_creation( | ||
cli_runner, db_session, enable_factory_create, clear_notification_logs, user | ||
): | ||
"""Test that notification logs are created when notifications are sent""" | ||
# Create a saved opportunity that needs notification | ||
opportunity = factories.OpportunityFactory.create() | ||
saved_opportunity = factories.UserSavedOpportunityFactory.create( | ||
user=user, | ||
opportunity=opportunity, | ||
last_notified_at=opportunity.updated_at - timedelta(days=1), | ||
) | ||
|
||
factories.OpportunityChangeAuditFactory.create( | ||
opportunity=opportunity, | ||
updated_at=saved_opportunity.last_notified_at + timedelta(minutes=1), | ||
) | ||
|
||
# Run the notification task | ||
result = cli_runner.invoke(args=["task", "generate-notifications"]) | ||
assert result.exit_code == 0 | ||
|
||
# Verify notification log was created | ||
notification_logs = db_session.query(UserNotificationLog).all() | ||
assert len(notification_logs) == 1 | ||
|
||
log = notification_logs[0] | ||
assert log.user_id == user.user_id | ||
assert log.notification_reason == NotificationConstants.OPPORTUNITY_UPDATES | ||
assert log.notification_sent is True | ||
|
||
|
||
def test_no_notification_log_when_no_updates( | ||
cli_runner, db_session, enable_factory_create, clear_notification_logs, user | ||
): | ||
"""Test that no notification log is created when there are no updates""" | ||
# Create a saved opportunity that doesn't need notification | ||
opportunity = factories.OpportunityFactory.create() | ||
factories.UserSavedOpportunityFactory.create( | ||
user=user, | ||
opportunity=opportunity, | ||
last_notified_at=opportunity.updated_at + timedelta(minutes=1), # After the update | ||
) | ||
|
||
# Run the notification task | ||
result = cli_runner.invoke(args=["task", "generate-notifications"]) | ||
assert result.exit_code == 0 | ||
|
||
# Verify no notification log was created | ||
notification_logs = db_session.query(UserNotificationLog).all() | ||
assert len(notification_logs) == 0 |
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want tests that are a bit less log-focused at the moment, I did add the notification table which we'll use as a sort of auditing table. Could start populating that with something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some new tests here to and adding logs in
generate_notifications.py