-
Notifications
You must be signed in to change notification settings - Fork 1
fix: create workspace_entities/workspace_documents registry and provision Workspace rows #1503
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
Draft
seonghobae
wants to merge
15
commits into
codex/starlette-testclient-dependency
Choose a base branch
from
fix/workspace-document-registry-migration
base: codex/starlette-testclient-dependency
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
4f8daef
fix: create workspace_entities/workspace_documents and provision Work…
claude 31b400c
fix(data): serialize workspace provisioning
seonghobae 37bcd6e
fix(data): bind documents to organization scope
seonghobae bfe347e
test: cover the workspace/organization collision in the quality surface
claude 317c721
fix: guard 0016 against a genuinely missing workspace_documents table
claude e05f1b3
test(data): reproduce legacy document organization scope regression
seonghobae 5054a8e
fix: trust legacy NULL-organization documents only for the owning org
claude 6fe1de1
fix: reconcile emails/email_records fix with PR #1502
claude 053c066
fix: guard 0011_email_read_state on column, not just table, existence
claude d3020ca
fix: make 0011_email_read_state's downgrade non-destructive
claude da2d7fe
fix: add forward migration repairing already-stamped is_read gap
claude 2beaf0e
fix: preserve document scope migration data
seonghobae 9c4e5ed
fix: create legacy index through SQLAlchemy
seonghobae 9c18513
fix: explain intentional best-effort DB teardown swallow
claude 19d5860
fix(migrations): inherit locked TestClient prerequisite
seonghobae 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
This file contains hidden or 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 hidden or 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 hidden or 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 hidden or 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,81 @@ | ||
| """create workspace registry and workspace document tables | ||
|
|
||
| Revision ID: 0018_workspace_registry | ||
| Revises: 0017_merge_newsdom_carddav_heads | ||
| Create Date: 2026-09-01 00:00:00.000000 | ||
|
|
||
| ``Workspace``/``Document`` (``workspace_entities``/``workspace_documents``) have | ||
| been declared in ``db/models.py`` since before this repository's incremental | ||
| migration history begins tracking them explicitly. A database that ran | ||
| ``0001_initial_control_plane``'s ``Base.metadata.create_all`` after these | ||
| models existed already has both tables; a database that ran ``0001`` earlier | ||
| and has only applied incremental migrations since never got them, so | ||
| ``/api/data/documents`` fails with an undefined-relation error the first time | ||
| it is hit. This revision is idempotent (``has_table`` guarded) so it is a | ||
| no-op for a database that already has the tables and a real fix for one that | ||
| does not. | ||
| """ | ||
|
|
||
| from alembic import op | ||
| import sqlalchemy as sa | ||
|
|
||
| revision = "0018_workspace_registry" | ||
| down_revision = "0017_merge_newsdom_carddav_heads" | ||
|
devin-ai-integration[bot] marked this conversation as resolved.
|
||
|
|
||
| _ENTITIES_TABLE = "workspace_entities" | ||
| _DOCUMENTS_TABLE = "workspace_documents" | ||
|
|
||
|
|
||
| def upgrade() -> None: | ||
| connection = op.get_bind() | ||
| inspector = sa.inspect(connection) | ||
|
|
||
| if not inspector.has_table(_ENTITIES_TABLE): | ||
| op.create_table( | ||
| _ENTITIES_TABLE, | ||
| sa.Column("workspace_id", sa.String(), nullable=False), | ||
| sa.Column("workspace_name", sa.String(), nullable=False), | ||
| sa.Column("workspace_domain", sa.String(), nullable=True), | ||
| sa.Column("created_at", sa.DateTime(timezone=True), nullable=False), | ||
| sa.PrimaryKeyConstraint("workspace_id"), | ||
| ) | ||
|
|
||
| if not inspector.has_table(_DOCUMENTS_TABLE): | ||
| op.create_table( | ||
| _DOCUMENTS_TABLE, | ||
| sa.Column("document_id", sa.String(), nullable=False), | ||
| sa.Column("workspace_id", sa.String(), nullable=False), | ||
| sa.Column("organization_id", sa.String(), nullable=True), | ||
| sa.Column("document_name", sa.String(), nullable=False), | ||
| sa.Column("document_type", sa.String(), nullable=False), | ||
| sa.Column("document_content", sa.Text(), nullable=True), | ||
| sa.Column("document_status", sa.String(), nullable=False), | ||
| sa.Column("created_at", sa.DateTime(timezone=True), nullable=False), | ||
| sa.ForeignKeyConstraint( | ||
| ["workspace_id"], [f"{_ENTITIES_TABLE}.workspace_id"] | ||
| ), | ||
| sa.PrimaryKeyConstraint("document_id"), | ||
| ) | ||
|
|
||
| for index_name, column_names in ( | ||
| ("ix_workspace_documents_workspace_id", ["workspace_id"]), | ||
| ("ix_workspace_documents_organization_id", ["organization_id"]), | ||
| ): | ||
| op.create_index( | ||
| index_name, | ||
| _DOCUMENTS_TABLE, | ||
| column_names, | ||
| if_not_exists=True, | ||
| ) | ||
|
|
||
|
|
||
| def downgrade() -> None: | ||
| # This revision's upgrade is a no-op whenever the tables already exist | ||
| # (e.g. created by 0001's create_all), so a downgrade cannot tell "this | ||
| # revision created these tables" apart from "they predate it" -- and | ||
| # workspace_documents.document_content holds real uploaded content, not | ||
| # rebuildable derived state. Unconditionally dropping it risks destroying | ||
| # data this revision never created. As with 0001_initial_control_plane: | ||
| # production rollbacks should restore from backup or a later explicit | ||
| # down revision rather than dropping customer-owned data. | ||
| return None | ||
This file contains hidden or 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,57 @@ | ||
| """idempotently ensure email_records has is_read | ||
|
|
||
| Revision ID: 0019_email_read_state_repair | ||
| Revises: 0018_workspace_registry | ||
| Create Date: 2026-09-01 00:00:00.000000 | ||
|
|
||
| Alembic never re-runs a revision's ``upgrade()`` once that revision id is | ||
| recorded as applied for a database -- editing ``0011_email_read_state.py``'s | ||
| content cannot repair a database that already has "0011_email_read_state" | ||
| in its ``alembic_version`` history but never actually got | ||
| ``email_records.is_read`` (whatever the reason: an earlier version of that | ||
| revision that targeted the wrong table, a partial/interrupted apply, manual | ||
| intervention). This revision is the real repair path for such a database: | ||
| appended after the current head, so it runs regardless of what 0011 already | ||
| did or didn't do. Idempotent (has_table/has_column guarded) so it is a | ||
| no-op for every database that already has the column, from any path. | ||
| """ | ||
|
|
||
| from alembic import op | ||
| import sqlalchemy as sa | ||
|
|
||
| revision = "0019_email_read_state_repair" | ||
| down_revision = "0018_workspace_registry" | ||
|
|
||
| _EMAIL_TABLE = "email_records" | ||
|
|
||
|
|
||
| def upgrade() -> None: | ||
| inspector = sa.inspect(op.get_bind()) | ||
| if inspector.has_table(_EMAIL_TABLE) and not _has_column( | ||
| inspector, _EMAIL_TABLE, "is_read" | ||
| ): | ||
| op.add_column( | ||
| _EMAIL_TABLE, | ||
| sa.Column( | ||
| "is_read", | ||
| sa.Boolean(), | ||
| nullable=False, | ||
| server_default=sa.text("true"), | ||
| ), | ||
| ) | ||
|
|
||
|
|
||
| def downgrade() -> None: | ||
| # Same ownership-ambiguity reasoning as 0011_email_read_state and | ||
| # 0018_workspace_registry: this revision cannot tell whether it was the | ||
| # one that added the column (repairing a stamped-but-incomplete | ||
| # database) or the column already existed from another path, and | ||
| # is_read holds real read/unread state, not rebuildable derived data. | ||
| # No-op; production rollbacks should restore from backup. | ||
| return None | ||
|
|
||
|
|
||
| def _has_column(inspector, table_name: str, column_name: str) -> bool: | ||
| return any( | ||
| column["name"] == column_name for column in inspector.get_columns(table_name) | ||
| ) |
This file contains hidden or 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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.