backend for onboarding messages

[AnnieeBennie]

• added a table for onboarding messages
• added a table for user&onboarding messages
• added migration for user&onboarding messages
• added the creation of rows for user&onboarding messages on account creation in user_account_creation.py
  (so the rows for all onboarding messages are added for user when the user creates account/basic account with time_shown null, after they are shown null changes to the time the message was shown).
• added api endpoints for onboarding

[github-actions]

ArchLens detected architectural changes in the following views:

[mircealungu]

Thanks Annie — solid first cut. A few merge-blockers and some cleanup before this is ready.

🔴 Must fix

1. get_onboarding_message_status will 500 on every call. It calls UserOnboardingMessage.has_message_shown_time(...) — that method is not defined in the model. Either add it, or change the endpoint to use find_by_user_and_message(...).message_shown_time is not None.

2. Authorization hole in set_onboarding_message_click_time. The endpoint accepts user_onboarding_message_id from the form and updates it with no ownership check. Any authenticated user can stamp click times on another user's records (the commented-out # user = User.find_by_id(...) is a tell). Either filter by (id, user_id=flask.g.user_id), or — cleaner — look up by (user_id, onboarding_message_id) and drop the surrogate id from the API.

3. update_user_onboarding_message_time crashes on a missing record. Unlike set_message_shown_time, there's no null-check before .message_click_time = .... Combined with #2, a bad id → 500.

🟡 Design

4. Pre-creating 7 rows per user at signup is unnecessary. The endpoint already uses find_or_create_for_user_and_message, which is the right pattern lazily. Drop the for msg_id in range(1, 8) loops in both branches of user_account_creation.py (lines 167-170 and 244-247). Bonus: avoids the magic range(1, 8) that silently breaks the day a message #8 is added.

5. Double commit. find_or_create_for_user_and_message calls session.commit() itself, and the endpoint then commits again. Helpers shouldn't commit — let the caller own the transaction boundary.

6. Endpoint name is misleading. get_onboarding_message_for_user is a POST that records the message was shown. Suggest mark_onboarding_message_shown.

🟢 Cleanup

• Migration filename convention. Project uses YY-MM-DD--<name>.sql (double-dash). Two of three are single-underscore — rename 26-05-03_add_onboarding_message_table.sql and 26-05-03_add_user_onboarding_message_table.sql. Also: fold the unique-constraint migration into the create-table migration since there's no prod data yet.
• Typo: onbaording → onboarding in onboarding_message.py.
• Delete the commented-out # user = User.find_by_id(flask.g.user_id) line.
• Trailing whitespace on from . import api, db_session and missing newline at EOF in three files.
• User.find_by_id(flask.g.user_id) in get_onboarding_message_status is a wasted query — flask.g.user_id is already the id.
• OnboardingMessage.id = Column(Integer, primary_key=True) mixes raw SQLAlchemy with db.Column elsewhere — pick one.
• db.Column(db.String) has no length; the migration is VARCHAR(45). Mirror as db.String(45).
• get_all_onboarding_messages_for_user has except NoResultFound around .all(), which never raises — dead code.
• OnboardingMessage.find_by_id catches generic Exception to Sentry — too broad; will mask real errors.
• No try/except ValueError on int(onboarding_message_id) — bad input → 500.

Tests

None added. At minimum I'd want:

• get_onboarding_message_status returns the right boolean (would have caught #1).
• Click-time endpoint refuses to update another user's record (would have caught #2).
• find-or-create idempotency.

The big one to discuss is #4 — happy to chat about it if the eager-creation was deliberate.

[AnnieeBennie]

@mircealungu I fixed that and created tests

[mircealungu]

@AnnieeBennie pushed a tiny follow-up (119a187) — same pattern fix you already applied to OnboardingMessage.find_by_id in 701fe0b, just mirrored to UserOnboardingMessage.find_by_id so a real DB error doesn't get swallowed into a misleading None. Tests still green locally. Ready to approve once CI is happy.