Add message tree state model #414
Conversation
|
debugging pre-commit |
| from sqlmodel import Field, Index, SQLModel | ||
|
|
||
| INITIAL = "initial" | ||
| BREEDING_PHASE = "breeding_phase" |
There was a problem hiding this comment.
these should probably be put into a str-enum (see other cases where enums are used in the prj)
There was a problem hiding this comment.
okay, Ill make the changes
There was a problem hiding this comment.
ok rewrote the Enum class for states and updated VALID_STATES
| "message_tree_state", | ||
| sa.Column("id", postgresql.UUID(as_uuid=True), server_default=sa.text("gen_random_uuid()"), nullable=False), | ||
| sa.Column("created_date", sa.DateTime(), server_default=sa.text("CURRENT_TIMESTAMP"), nullable=False), | ||
| sa.Column("deleted", sa.Boolean(), server_default=sa.text("false"), nullable=False), |
There was a problem hiding this comment.
not sure about the deleted flag here, that should already be part of message, right?
There was a problem hiding this comment.
I apologize, I made the proper changes to remove it. this pull request now adds two alembic revisions. should I restart the pull request to resolve this?
| op.create_table( | ||
| "message_tree_state", | ||
| sa.Column("id", postgresql.UUID(as_uuid=True), server_default=sa.text("gen_random_uuid()"), nullable=False), | ||
| sa.Column("created_date", sa.DateTime(), server_default=sa.text("CURRENT_TIMESTAMP"), nullable=False), |
There was a problem hiding this comment.
created_date is also already part of message .. so would be redundant. I think we should remove/not add it here.
| # The types of States a message tree can have. | ||
|
|
||
|
|
||
| class States(Enum): |
There was a problem hiding this comment.
| class States(Enum): | |
| class States(str, Enum): |
There was a problem hiding this comment.
No problem
| VALID_STATES = ( | ||
| States.INITIAL, | ||
| States.BREEDING_PHASE, | ||
| States.RANKING_PHASE, | ||
| States.READY_FOR_SCORING, | ||
| States.CHILDREN_SCORED, | ||
| States.FINAL, | ||
| ) |
There was a problem hiding this comment.
could this be replaced by either tuple(States) or tuple(s.value for s in States) ?
This commit is part of issue #37
This PR won't break anything if merged. However, message_tree_state isn't functional i.e. adding a new message won't initialize a tree state.