Skip to content
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

Remove all advisor stuff and fix migrations #25

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

tmyracle
Copy link
Contributor

Addresses #21

  • Removes all advisor functionality
  • Removed relevant models and enums from database:
    • Advisor
    • Conversation
    • ConversationAdvisor
    • ConversationNote
    • MessageType
    • Message
    • ConversationStatus
    • ata fields from notifications on User

The notifications preferences UI will likely need an update (or total removal) later since the only thing left after removing the advisor notifications is a ConverKit function or 2. Will revisit once we get auth sorted and can log in and really test the UI.

I also tried to clean up some unused imports throughout files I touched, hence some of the deletions there.

This change also uses introspection to populate the dbgenerated parameters for columns that are a custom type (non native data type). There's a known issue in Prisma where dbgenerated() with nothing passed in will continue to generate migrations that are not compatible when it should noop. This should fix the migration hiccups discussed in the Discord.

I don't love having those big database functions in the schema.prisma file but seems to be the best workaround to get migrations working smoothly again. Might revisit later and refactor using triggers.

I also bumped prisma dependency to see if they had patched the issue, but no luck.

Lots of stuff going on here to remove all the advisor functionality so apologies for the massive PR.

@Shpigford Shpigford merged commit d59484a into maybe-finance:main Jan 11, 2024
1 check failed
@Shpigford
Copy link
Member

Thanks @tmyracle! Definitely would prefer smaller PRs in the future, but fine at the moment given we're doing some major cleanup and trying to reduce the overall complexity of the app.

Have merged to main.

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.

2 participants