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.

Sign up for GitHub

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

Jump to bottom

DB structure/remove unnecessary labels field #413

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

DB structure/remove unnecessary labels field #413

wants to merge 7 commits into from

Conversation

PragTob
Copy link
Collaborator

@PragTob PragTob commented Oct 5, 2024

Should fix #367 although it wasn't label_id but label. I didn't find any obvious others, but also I'm not as familiar with the app as y'all.

Further Notes

  • deleted the 2 year and 4 year old db migration scripts along their tests, we don't need them and also they broke :)
  • this is not a safe migration, while the migrations runs during deployment the app will be broken. I'm not sure if that's fine as I'm not sure how much traffic the app has. Safe way would be to deploy the code changes and then deploy the migration separately, we can still split that up.

@PragTob PragTob changed the title Db structure DB structure/remove unnecessary labels field Oct 5, 2024
@JannikStreek
Copy link
Member

A small downtime is not an issue, thanks for cleaning up. I will test it locally first :)

@JannikStreek
Copy link
Member

I think this can be removed as well (and from schema):

belongs_to :label, IdeaLabel, foreign_key: :label_id, type: :binary_id, on_replace: :nilify

as we only use many_to_many :idea_labels now. But please double check that 😅

@PragTob
Remove idea.label
b27ac77 
we're using the many_to_many idea_labels:
#413 (comment)
@PragTob
Copy link
Collaborator Author

PragTob commented Nov 3, 2024 •
edited
Loading

Yup removed that one as well and it seems to work 🎉

@JannikStreek please re-review

@JannikStreek
Copy link
Member

@PragTob can you solve the conflicts ? 😅 sorry

@PragTob
Copy link
Collaborator Author

PragTob commented Nov 11, 2024

@JannikStreek

ok fine

@PragTob
Copy link
Collaborator Author

PragTob commented Nov 13, 2024

@JannikStreek should be good! Test may fail because the flakey fix isn't merged yet but yeah :)

@PragTob
Copy link
Collaborator Author

PragTob commented Nov 13, 2024

wait what:

SELECT b0."id", b0."name", b0."option_show_link_to_settings", b0."option_allow_manual_ordering", b0."admin_url_id", b0."last_accessed_at", b0."filter_labels_ids", b0."creating_user_id", b0."inserted_at", b0."updated_at" FROM "brainstormings" AS b0 INNER JOIN (SELECT DISTINCT ON (si0."brainstorming_id") si0."id" AS "id", si0."body" AS "body", si0."position_order" AS "position_order", si0."username" AS "username", si0."label" AS "deprecated_label", si0."user_id" AS "user_id", si0."brainstorming_id" AS "brainstorming_id", si0."label_id" AS "label_id", si0."lane_id" AS "lane_id", si0."inserted_at" AS "inserted_at", si0."updated_at" AS "updated_at" FROM "ideas" AS si0 WHERE (si0."position_order" IS NULL)) AS s1 ON s1."brainstorming_id" = b0."id" []
** (Postgrex.Error) ERROR 42703 (undefined_column) column si0.label does not exist

    query: SELECT b0."id", b0."name", b0."option_show_link_to_settings", b0."option_allow_manual_ordering", b0."admin_url_id", b0."last_accessed_at", b0."filter_labels_ids", b0."creating_user_id", b0."inserted_at", b0."updated_at" FROM "brainstormings" AS b0 INNER JOIN (SELECT DISTINCT ON (si0."brainstorming_id") si0."id" AS "id", si0."body" AS "body", si0."position_order" AS "position_order", si0."username" AS "username", si0."label" AS "deprecated_label", si0."user_id" AS "user_id", si0."brainstorming_id" AS "brainstorming_id", si0."label_id" AS "label_id", si0."lane_id" AS "lane_id", si0."inserted_at" AS "inserted_at", si0."updated_at" AS "updated_at" FROM "ideas" AS si0 WHERE (si0."position_order" IS NULL)) AS s1 ON s1."brainstorming_id" = b0."id"
    (ecto_sql 3.12.1) lib/ecto/adapters/sql.ex:1096: Ecto.Adapters.SQL.raise_sql_call_error/1
    (ecto_sql 3.12.1) lib/ecto/adapters/sql.ex:994: Ecto.Adapters.SQL.execute/6
    (ecto 3.12.4) lib/ecto/repo/queryable.ex:232: Ecto.Repo.Queryable.execute/4
    (ecto 3.12.4) lib/ecto/repo/queryable.ex:19: Ecto.Repo.Queryable.all/3
    _build/test/lib/mindwendel/priv/repo/data_migrations/migrate_add_position_order_to_ideas.exs:73: Mindwendel.Repo.DataMigrations.MigrateAddPositionOrderToIdeas.run/0
    (ecto_sql 3.12.1) lib/ecto/migration/runner.ex:310: Ecto.Migration.Runner.perform_operation/3
    (stdlib 5.2.3) timer.erl:270: :timer.tc/2
    (ecto_sql 3.12.1) lib/ecto/migration/runner.ex:25: Ecto.Migration.Runner.run/8

@PragTob
Copy link
Collaborator Author

PragTob commented Nov 13, 2024

I think I added a migration so long ago that a newer migration uses the fields I wanted to remove, as the runner wants to run them in their time order.... best fix is probably to regenerate the mgirations

@PragTob
Copy link
Collaborator Author

PragTob commented Nov 13, 2024

Aight, that works!

@PragTob
Copy link
Collaborator Author

PragTob commented Nov 17, 2024

@JannikStreek review please, I don't wanna keep fighting merge conflicts here 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

chore: cleanup database structure
2 participants
@PragTob @JannikStreek