Add seen tables by sssoleileraaa · Pull Request #1164 · freedomofpress/securedrop-client

sssoleileraaa · 2020-10-21T06:49:10Z

Description

Fixes #1163
(In draft mode until #1162 is merged)

Test Plan

rm -r /tmp/sdc-*
TESTS=tests/test_alembic.py::test_alembic_migration_upgrade_with_data make test
sqlite3 /tmp/sdc-<random-string>/svs.sqlite

query all three seen tables and see that they exist

repeat steps 1-2 above but for the downgrade test

seen tables no longer exist

kushaldas

HEAD is at

✦ ❯ git log
commit aa8e485531f6e6976131a3131e164edf5cbfcd11 (HEAD -> add-seen-tables, origin/add-seen-tables)
Author: Allie Crevier <[email protected]>
Date:   Wed Oct 21 18:30:31 2020 -0700

    add seen tables and test migration

The test command TESTS=tests/test_alembic.py::test_alembic_migration_upgrade_with_data make test passes,
but none of the seen tables is seen in the database.

✦ ❯ sqlite3 /tmp/sdc-vywvim7u/svs.sqlite 
SQLite version 3.27.2 2019-02-25 16:06:06
Enter ".help" for usage hints.
sqlite> .schema
CREATE TABLE alembic_version (
	version_num VARCHAR(32) NOT NULL, 
	CONSTRAINT alembic_version_pkc PRIMARY KEY (version_num)
);
CREATE TABLE sources (
	id INTEGER NOT NULL, 
	uuid VARCHAR(36) NOT NULL, 
	journalist_designation VARCHAR(255) NOT NULL, 
	document_count INTEGER DEFAULT 0 NOT NULL, 
	is_flagged BOOLEAN DEFAULT 0, 
	public_key TEXT, 
	fingerprint VARCHAR(64), 
	interaction_count INTEGER DEFAULT 0 NOT NULL, 
	is_starred BOOLEAN DEFAULT 0, 
	last_updated DATETIME, 
	CONSTRAINT pk_sources PRIMARY KEY (id), 
	CONSTRAINT uq_sources_uuid UNIQUE (uuid), 
	CONSTRAINT ck_sources_is_flagged CHECK (is_flagged IN (0, 1)), 
	CONSTRAINT ck_sources_is_starred CHECK (is_starred IN (0, 1))
);
CREATE TABLE users (
	id INTEGER NOT NULL, 
	uuid VARCHAR(36) NOT NULL, 
	username VARCHAR(255) NOT NULL, firstname VARCHAR(64), lastname VARCHAR(64), 
	CONSTRAINT pk_users PRIMARY KEY (id), 
	CONSTRAINT uq_users_uuid UNIQUE (uuid)
);
CREATE TABLE replysendstatuses (
	id INTEGER NOT NULL, 
	name VARCHAR(36) NOT NULL, 
	CONSTRAINT pk_replysendstatuses PRIMARY KEY (id), 
	CONSTRAINT uq_replysendstatuses_name UNIQUE (name)
);
CREATE TABLE draftreplies (
	id INTEGER NOT NULL, 
	uuid VARCHAR(36) NOT NULL, 
	timestamp DATETIME NOT NULL, 
	source_id INTEGER NOT NULL, 
	journalist_id INTEGER, 
	file_counter INTEGER NOT NULL, 
	content TEXT, 
	send_status_id INTEGER, 
	CONSTRAINT pk_draftreplies PRIMARY KEY (id), 
	CONSTRAINT uq_draftreplies_uuid UNIQUE (uuid), 
	CONSTRAINT fk_draftreplies_source_id_sources FOREIGN KEY(source_id) REFERENCES sources (id), 
	CONSTRAINT fk_draftreplies_journalist_id_users FOREIGN KEY(journalist_id) REFERENCES users (id), 
	CONSTRAINT fk_draftreplies_send_status_id_replysendstatuses FOREIGN KEY(send_status_id) REFERENCES replysendstatuses (id)
);
CREATE TABLE downloaderrors (
	id INTEGER NOT NULL, 
	name VARCHAR(36) NOT NULL, 
	CONSTRAINT pk_downloaderrors PRIMARY KEY (id), 
	CONSTRAINT uq_downloaderrors_name UNIQUE (name)
);
CREATE TABLE files (
        id INTEGER NOT NULL,
        uuid VARCHAR(36) NOT NULL,
        filename VARCHAR(255) NOT NULL,
        file_counter INTEGER NOT NULL,
        size INTEGER NOT NULL,
        download_url VARCHAR(255) NOT NULL,
        is_downloaded BOOLEAN DEFAULT 0 NOT NULL,
        is_decrypted BOOLEAN CONSTRAINT files_compare_is_downloaded_vs_is_decrypted CHECK (CASE WHEN is_downloaded = 0 THEN is_decrypted IS NULL ELSE 1 END),
        download_error_id INTEGER,
        is_read BOOLEAN DEFAULT 0 NOT NULL,
        source_id INTEGER NOT NULL,
        last_updated DATETIME NOT NULL,
        CONSTRAINT pk_files PRIMARY KEY (id),
        CONSTRAINT uq_messages_source_id_file_counter UNIQUE (source_id, file_counter),
        CONSTRAINT uq_files_uuid UNIQUE (uuid),
        CONSTRAINT ck_files_is_downloaded CHECK (is_downloaded IN (0, 1)),
        CONSTRAINT ck_files_is_decrypted CHECK (is_decrypted IN (0, 1)),
        CONSTRAINT fk_files_download_error_id_downloaderrors FOREIGN KEY(download_error_id) REFERENCES downloaderrors (id),
        CONSTRAINT ck_files_is_read CHECK (is_read IN (0, 1)),
        CONSTRAINT fk_files_source_id_sources FOREIGN KEY(source_id) REFERENCES sources (id)
);
CREATE TABLE messages (
        id INTEGER NOT NULL,
        uuid VARCHAR(36) NOT NULL,
        filename VARCHAR(255) NOT NULL,
        file_counter INTEGER NOT NULL,
        size INTEGER NOT NULL,
        download_url VARCHAR(255) NOT NULL,
        is_downloaded BOOLEAN DEFAULT 0 NOT NULL,
        is_decrypted BOOLEAN CONSTRAINT messages_compare_is_downloaded_vs_is_decrypted CHECK (CASE WHEN is_downloaded = 0 THEN is_decrypted IS NULL ELSE 1 END),
        download_error_id INTEGER,
        is_read BOOLEAN DEFAULT 0 NOT NULL,
        content TEXT CONSTRAINT ck_message_compare_download_vs_content CHECK (CASE WHEN is_downloaded = 0 THEN content IS NULL ELSE 1 END),
        source_id INTEGER NOT NULL,
        last_updated DATETIME NOT NULL,
        CONSTRAINT pk_messages PRIMARY KEY (id),
        CONSTRAINT uq_messages_source_id_file_counter UNIQUE (source_id, file_counter),
        CONSTRAINT uq_messages_uuid UNIQUE (uuid),
        CONSTRAINT ck_messages_is_downloaded CHECK (is_downloaded IN (0, 1)),
        CONSTRAINT ck_messages_is_decrypted CHECK (is_decrypted IN (0, 1)),
        CONSTRAINT fk_messages_download_error_id_downloaderrors FOREIGN KEY(download_error_id) REFERENCES downloaderrors (id),
        CONSTRAINT ck_messages_is_read CHECK (is_read IN (0, 1)),
        CONSTRAINT fk_messages_source_id_sources FOREIGN KEY(source_id) REFERENCES sources (id)
    );
CREATE TABLE replies (
        id INTEGER NOT NULL,
        uuid VARCHAR(36) NOT NULL,
        source_id INTEGER NOT NULL,
        filename VARCHAR(255) NOT NULL,
        file_counter INTEGER NOT NULL,
        size INTEGER,
        content TEXT,
        is_decrypted BOOLEAN,
        is_downloaded BOOLEAN,
        download_error_id INTEGER,
        journalist_id INTEGER,
        last_updated DATETIME NOT NULL,
        CONSTRAINT pk_replies PRIMARY KEY (id),
        CONSTRAINT uq_messages_source_id_file_counter UNIQUE (source_id, file_counter),
        CONSTRAINT uq_replies_uuid UNIQUE (uuid),
        CONSTRAINT fk_replies_source_id_sources FOREIGN KEY(source_id) REFERENCES sources (id),
        CONSTRAINT fk_replies_download_error_id_downloaderrors
            FOREIGN KEY(download_error_id) REFERENCES downloaderrors (id),
        CONSTRAINT fk_replies_journalist_id_users FOREIGN KEY(journalist_id) REFERENCES users (id),
        CONSTRAINT replies_compare_download_vs_content
            CHECK (CASE WHEN is_downloaded = 0 THEN content IS NULL ELSE 1 END),
        CONSTRAINT replies_compare_is_downloaded_vs_is_decrypted
            CHECK (CASE WHEN is_downloaded = 0 THEN is_decrypted IS NULL ELSE 1 END),
        CONSTRAINT ck_replies_is_decrypted CHECK (is_decrypted IN (0, 1)),
        CONSTRAINT ck_replies_is_downloaded CHECK (is_downloaded IN (0, 1))
    );

After digging more into the migrations and the test, I found the following migrations are being applied
into the database during the test.

tests/test_alembic.py::test_alembic_migration_upgrade_with_data[a4bf1f58ce69]
INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade  -> 2f363b3d680e, init
INFO  [alembic.runtime.migration] Running upgrade 2f363b3d680e -> fecf1191b6f0, remove decryption_vs_content contraint
INFO  [alembic.runtime.migration] Running upgrade fecf1191b6f0 -> bafdcae12f97, Add files.original_filename
INFO  [alembic.runtime.migration] Running upgrade bafdcae12f97 -> 36a79ffcfbfb, add first_name, last_name, fullname, initials
INFO  [alembic.runtime.migration] Running upgrade 36a79ffcfbfb -> 86b01b6290da, add reply draft
INFO  [alembic.runtime.migration] Running upgrade 86b01b6290da -> fb657f2ee8a7, drop File.original_filename
INFO  [alembic.runtime.migration] Running upgrade fb657f2ee8a7 -> 7f682532afa2, add download error
main: /tmp/sdc-jnwhx3r2/svs.sqlite
INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade 7f682532afa2 -> a4bf1f58ce69, fix journalist association in replies table
PASSED

This is because the seen tables are in bd57477f19a2 revision, but DATA_MIGRATIONS variable is pointing to a4bf1f58ce69, which is
the previous revision. If I manually update the migration revision in the test using the following patch, I get a new set of errors, but the seen tables are now present.

diff --git a/tests/test_alembic.py b/tests/test_alembic.py
index 2a406dc..e142692 100644
--- a/tests/test_alembic.py
+++ b/tests/test_alembic.py
@@ -137,7 +137,7 @@ def test_alembic_migration_upgrade(alembic_config, config, migration):
         upgrade(alembic_config, mig)
 
 
-@pytest.mark.parametrize("migration", DATA_MIGRATIONS)
+@pytest.mark.parametrize("migration", ["bd57477f19a2",])
 def test_alembic_migration_upgrade_with_data(alembic_config, config, migration, homedir):
     """
     Upgrade to one migration before the target migration, load data, then upgrade in order to test
@@ -146,6 +146,7 @@ def test_alembic_migration_upgrade_with_data(alembic_config, config, migration,
     migrations = list_migrations(alembic_config, migration)
     if len(migrations) == 1:
         return
+    print("reached here")
     upgrade(alembic_config, migrations[-2])
     mod_name = "tests.migrations.test_{}".format(migration)
     mod = __import__(mod_name, fromlist=["UpgradeTester"])

The test command and the error:

 python -m pytest -vv -s tests/test_alembic.py::test_alembic_migration_upgrade_with_data 
============================================================== test session starts ===============================================================
platform linux -- Python 3.7.3, pytest-5.2.1, py-1.7.0, pluggy-0.13.0 -- /home/kdas/code/securedrop-client/.venv/bin/python
cachedir: .pytest_cache
PyQt5 5.11.3 -- Qt runtime 5.11.2 -- Qt compiled 5.11.2
Test order randomisation NOT enabled. Enable with --random-order or --random-order-bucket=<bucket_type>
rootdir: /home/kdas/code/securedrop-client, inifile: pytest.ini
plugins: qt-3.3.0, xdist-1.30.0, mock-1.10.0, forked-1.1.3, flaky-3.6.1, cov-2.8.1, vcr-1.0.2, random-order-1.0.4
collected 1 item                                                                                                                                 

tests/test_alembic.py::test_alembic_migration_upgrade_with_data[bd57477f19a2] reached here
INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade  -> 2f363b3d680e, init
INFO  [alembic.runtime.migration] Running upgrade 2f363b3d680e -> fecf1191b6f0, remove decryption_vs_content contraint
INFO  [alembic.runtime.migration] Running upgrade fecf1191b6f0 -> bafdcae12f97, Add files.original_filename
INFO  [alembic.runtime.migration] Running upgrade bafdcae12f97 -> 36a79ffcfbfb, add first_name, last_name, fullname, initials
INFO  [alembic.runtime.migration] Running upgrade 36a79ffcfbfb -> 86b01b6290da, add reply draft
INFO  [alembic.runtime.migration] Running upgrade 86b01b6290da -> fb657f2ee8a7, drop File.original_filename
INFO  [alembic.runtime.migration] Running upgrade fb657f2ee8a7 -> 7f682532afa2, add download error
INFO  [alembic.runtime.migration] Running upgrade 7f682532afa2 -> a4bf1f58ce69, fix journalist association in replies table
main: /tmp/sdc-ng2c5tmw/svs.sqlite
INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade a4bf1f58ce69 -> bd57477f19a2, add seen tables
FAILED

==================================================================== FAILURES ====================================================================
_____________________________________________ test_alembic_migration_upgrade_with_data[bd57477f19a2] _____________________________________________

alembic_config = '/tmp/sdc-ng2c5tmw/alembic.ini', config = '/tmp/sdc-ng2c5tmw/config.json', migration = 'bd57477f19a2'
homedir = '/tmp/sdc-ng2c5tmw'

    @pytest.mark.parametrize("migration", ["bd57477f19a2",])
    def test_alembic_migration_upgrade_with_data(alembic_config, config, migration, homedir):
        """
        Upgrade to one migration before the target migration, load data, then upgrade in order to test
        that the upgrade is successful when there is data.
        """
        migrations = list_migrations(alembic_config, migration)
        if len(migrations) == 1:
            return
        print("reached here")
        upgrade(alembic_config, migrations[-2])
        mod_name = "tests.migrations.test_{}".format(migration)
        mod = __import__(mod_name, fromlist=["UpgradeTester"])
        upgrade_tester = mod.UpgradeTester(homedir)
        upgrade_tester.load_data()
        upgrade(alembic_config, migration)
>       upgrade_tester.check_upgrade()

tests/test_alembic.py:156: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
tests/migrations/test_bd57477f19a2.py:85: in check_upgrade
    self.session.query(User).filter_by(id=reply.journalist_id).one()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <sqlalchemy.orm.query.Query object at 0x7f24e7e1bcc0>

    def one(self):
        """Return exactly one result or raise an exception.
    
        Raises ``sqlalchemy.orm.exc.NoResultFound`` if the query selects
        no rows.  Raises ``sqlalchemy.orm.exc.MultipleResultsFound``
        if multiple object identities are returned, or if multiple
        rows are returned for a query that returns only scalar values
        as opposed to full identity-mapped entities.
    
        Calling :meth:`.one` results in an execution of the underlying query.
    
        .. seealso::
    
            :meth:`.Query.first`
    
            :meth:`.Query.one_or_none`
    
        """
        try:
            ret = self.one_or_none()
        except orm_exc.MultipleResultsFound:
            raise orm_exc.MultipleResultsFound(
                "Multiple rows were found for one()"
            )
        else:
            if ret is None:
>               raise orm_exc.NoResultFound("No row was found for one()")
E               sqlalchemy.orm.exc.NoResultFound: No row was found for one()

.venv/lib/python3.7/site-packages/sqlalchemy/orm/query.py:3282: NoResultFound

The database schema:

SQLite version 3.27.2 2019-02-25 16:06:06
Enter ".help" for usage hints.
sqlite> .schema
CREATE TABLE alembic_version (
	version_num VARCHAR(32) NOT NULL, 
	CONSTRAINT alembic_version_pkc PRIMARY KEY (version_num)
);
CREATE TABLE sources (
	id INTEGER NOT NULL, 
	uuid VARCHAR(36) NOT NULL, 
	journalist_designation VARCHAR(255) NOT NULL, 
	document_count INTEGER DEFAULT 0 NOT NULL, 
	is_flagged BOOLEAN DEFAULT 0, 
	public_key TEXT, 
	fingerprint VARCHAR(64), 
	interaction_count INTEGER DEFAULT 0 NOT NULL, 
	is_starred BOOLEAN DEFAULT 0, 
	last_updated DATETIME, 
	CONSTRAINT pk_sources PRIMARY KEY (id), 
	CONSTRAINT uq_sources_uuid UNIQUE (uuid), 
	CONSTRAINT ck_sources_is_flagged CHECK (is_flagged IN (0, 1)), 
	CONSTRAINT ck_sources_is_starred CHECK (is_starred IN (0, 1))
);
CREATE TABLE users (
	id INTEGER NOT NULL, 
	uuid VARCHAR(36) NOT NULL, 
	username VARCHAR(255) NOT NULL, firstname VARCHAR(64), lastname VARCHAR(64), 
	CONSTRAINT pk_users PRIMARY KEY (id), 
	CONSTRAINT uq_users_uuid UNIQUE (uuid)
);
CREATE TABLE replysendstatuses (
	id INTEGER NOT NULL, 
	name VARCHAR(36) NOT NULL, 
	CONSTRAINT pk_replysendstatuses PRIMARY KEY (id), 
	CONSTRAINT uq_replysendstatuses_name UNIQUE (name)
);
CREATE TABLE draftreplies (
	id INTEGER NOT NULL, 
	uuid VARCHAR(36) NOT NULL, 
	timestamp DATETIME NOT NULL, 
	source_id INTEGER NOT NULL, 
	journalist_id INTEGER, 
	file_counter INTEGER NOT NULL, 
	content TEXT, 
	send_status_id INTEGER, 
	CONSTRAINT pk_draftreplies PRIMARY KEY (id), 
	CONSTRAINT uq_draftreplies_uuid UNIQUE (uuid), 
	CONSTRAINT fk_draftreplies_source_id_sources FOREIGN KEY(source_id) REFERENCES sources (id), 
	CONSTRAINT fk_draftreplies_journalist_id_users FOREIGN KEY(journalist_id) REFERENCES users (id), 
	CONSTRAINT fk_draftreplies_send_status_id_replysendstatuses FOREIGN KEY(send_status_id) REFERENCES replysendstatuses (id)
);
CREATE TABLE downloaderrors (
	id INTEGER NOT NULL, 
	name VARCHAR(36) NOT NULL, 
	CONSTRAINT pk_downloaderrors PRIMARY KEY (id), 
	CONSTRAINT uq_downloaderrors_name UNIQUE (name)
);
CREATE TABLE files (
        id INTEGER NOT NULL,
        uuid VARCHAR(36) NOT NULL,
        filename VARCHAR(255) NOT NULL,
        file_counter INTEGER NOT NULL,
        size INTEGER NOT NULL,
        download_url VARCHAR(255) NOT NULL,
        is_downloaded BOOLEAN DEFAULT 0 NOT NULL,
        is_decrypted BOOLEAN CONSTRAINT files_compare_is_downloaded_vs_is_decrypted CHECK (CASE WHEN is_downloaded = 0 THEN is_decrypted IS NULL ELSE 1 END),
        download_error_id INTEGER,
        is_read BOOLEAN DEFAULT 0 NOT NULL,
        source_id INTEGER NOT NULL,
        last_updated DATETIME NOT NULL,
        CONSTRAINT pk_files PRIMARY KEY (id),
        CONSTRAINT uq_messages_source_id_file_counter UNIQUE (source_id, file_counter),
        CONSTRAINT uq_files_uuid UNIQUE (uuid),
        CONSTRAINT ck_files_is_downloaded CHECK (is_downloaded IN (0, 1)),
        CONSTRAINT ck_files_is_decrypted CHECK (is_decrypted IN (0, 1)),
        CONSTRAINT fk_files_download_error_id_downloaderrors FOREIGN KEY(download_error_id) REFERENCES downloaderrors (id),
        CONSTRAINT ck_files_is_read CHECK (is_read IN (0, 1)),
        CONSTRAINT fk_files_source_id_sources FOREIGN KEY(source_id) REFERENCES sources (id)
);
CREATE TABLE messages (
        id INTEGER NOT NULL,
        uuid VARCHAR(36) NOT NULL,
        filename VARCHAR(255) NOT NULL,
        file_counter INTEGER NOT NULL,
        size INTEGER NOT NULL,
        download_url VARCHAR(255) NOT NULL,
        is_downloaded BOOLEAN DEFAULT 0 NOT NULL,
        is_decrypted BOOLEAN CONSTRAINT messages_compare_is_downloaded_vs_is_decrypted CHECK (CASE WHEN is_downloaded = 0 THEN is_decrypted IS NULL ELSE 1 END),
        download_error_id INTEGER,
        is_read BOOLEAN DEFAULT 0 NOT NULL,
        content TEXT CONSTRAINT ck_message_compare_download_vs_content CHECK (CASE WHEN is_downloaded = 0 THEN content IS NULL ELSE 1 END),
        source_id INTEGER NOT NULL,
        last_updated DATETIME NOT NULL,
        CONSTRAINT pk_messages PRIMARY KEY (id),
        CONSTRAINT uq_messages_source_id_file_counter UNIQUE (source_id, file_counter),
        CONSTRAINT uq_messages_uuid UNIQUE (uuid),
        CONSTRAINT ck_messages_is_downloaded CHECK (is_downloaded IN (0, 1)),
        CONSTRAINT ck_messages_is_decrypted CHECK (is_decrypted IN (0, 1)),
        CONSTRAINT fk_messages_download_error_id_downloaderrors FOREIGN KEY(download_error_id) REFERENCES downloaderrors (id),
        CONSTRAINT ck_messages_is_read CHECK (is_read IN (0, 1)),
        CONSTRAINT fk_messages_source_id_sources FOREIGN KEY(source_id) REFERENCES sources (id)
    );
CREATE TABLE replies (
        id INTEGER NOT NULL,
        uuid VARCHAR(36) NOT NULL,
        source_id INTEGER NOT NULL,
        filename VARCHAR(255) NOT NULL,
        file_counter INTEGER NOT NULL,
        size INTEGER,
        content TEXT,
        is_decrypted BOOLEAN,
        is_downloaded BOOLEAN,
        download_error_id INTEGER,
        journalist_id INTEGER,
        last_updated DATETIME NOT NULL,
        CONSTRAINT pk_replies PRIMARY KEY (id),
        CONSTRAINT uq_messages_source_id_file_counter UNIQUE (source_id, file_counter),
        CONSTRAINT uq_replies_uuid UNIQUE (uuid),
        CONSTRAINT fk_replies_source_id_sources FOREIGN KEY(source_id) REFERENCES sources (id),
        CONSTRAINT fk_replies_download_error_id_downloaderrors
            FOREIGN KEY(download_error_id) REFERENCES downloaderrors (id),
        CONSTRAINT fk_replies_journalist_id_users FOREIGN KEY(journalist_id) REFERENCES users (id),
        CONSTRAINT replies_compare_download_vs_content
            CHECK (CASE WHEN is_downloaded = 0 THEN content IS NULL ELSE 1 END),
        CONSTRAINT replies_compare_is_downloaded_vs_is_decrypted
            CHECK (CASE WHEN is_downloaded = 0 THEN is_decrypted IS NULL ELSE 1 END),
        CONSTRAINT ck_replies_is_decrypted CHECK (is_decrypted IN (0, 1)),
        CONSTRAINT ck_replies_is_downloaded CHECK (is_downloaded IN (0, 1))
    );
CREATE TABLE seen_files (
	id INTEGER NOT NULL, 
	file_id INTEGER NOT NULL, 
	journalist_id INTEGER, 
	CONSTRAINT pk_seen_files PRIMARY KEY (id), 
	CONSTRAINT fk_seen_files_file_id_files FOREIGN KEY(file_id) REFERENCES files (id), 
	CONSTRAINT fk_seen_files_journalist_id_users FOREIGN KEY(journalist_id) REFERENCES users (id), 
	CONSTRAINT uq_seen_files_file_id UNIQUE (file_id, journalist_id)
);
CREATE TABLE seen_messages (
	id INTEGER NOT NULL, 
	message_id INTEGER NOT NULL, 
	journalist_id INTEGER, 
	CONSTRAINT pk_seen_messages PRIMARY KEY (id), 
	CONSTRAINT fk_seen_messages_journalist_id_users FOREIGN KEY(journalist_id) REFERENCES users (id), 
	CONSTRAINT fk_seen_messages_message_id_messages FOREIGN KEY(message_id) REFERENCES messages (id), 
	CONSTRAINT uq_seen_messages_message_id UNIQUE (message_id, journalist_id)
);
CREATE TABLE seen_replies (
	id INTEGER NOT NULL, 
	reply_id INTEGER NOT NULL, 
	journalist_id INTEGER, 
	CONSTRAINT pk_seen_replies PRIMARY KEY (id), 
	CONSTRAINT fk_seen_replies_journalist_id_users FOREIGN KEY(journalist_id) REFERENCES users (id), 
	CONSTRAINT fk_seen_replies_reply_id_replies FOREIGN KEY(reply_id) REFERENCES replies (id), 
	CONSTRAINT uq_seen_replies_reply_id UNIQUE (reply_id, journalist_id)
);

`

sssoleileraaa · 2020-11-04T17:54:07Z

I see the same error when I run test_alembic_migration_upgrade_with_data[bd57477f19a2] which we weren't seeing fail in CI earlier because it wasn't included. Now that it's included, you can see the same issue you discovered in CI.

This is the first time we'll be using the new migration test code with more than one migration, so time for a little debugging since the issue is likely within that code (not with the seen-tables migration itself since it's quite simple and works if you run through the test plan).

sssoleileraaa · 2020-11-04T18:28:01Z

Addressed Kushal's comment above. Pushing it back to ready for review.

emkll

Test plan looks good to me, and @kushaldas ' findings seem to be addressed in the latest revision. Tested as follows:

upgrade revision: query all three seen tables and see that they exist
downgrade revision: seen tables no longer exist
starting the client locally and inspecting the database.
a deb package on this branch upgrade the client in sd-app with a populated database and inspecting the local database.

Functionally, the changes here look good to me, I have a few questions inline.

emkll

LGTM, thanks !

Comments have been addressed, test plan is now passing

sssoleileraaa force-pushed the add-seen-tables branch from 9acea2a to a9c318e Compare October 22, 2020 00:59

sssoleileraaa mentioned this pull request Oct 22, 2020

Add Seen/Unseen support at source level #1165

Merged

sssoleileraaa force-pushed the add-seen-tables branch 2 times, most recently from 6e0cd4a to b9e67cf Compare October 26, 2020 17:44

sssoleileraaa force-pushed the add-seen-tables branch from b9e67cf to aa8e485 Compare November 3, 2020 00:13

sssoleileraaa marked this pull request as ready for review November 3, 2020 00:13

sssoleileraaa requested review from kushaldas and redshiftzero as code owners November 3, 2020 00:13

kushaldas previously requested changes Nov 4, 2020

View reviewed changes

sssoleileraaa force-pushed the add-seen-tables branch 2 times, most recently from d2bdf0b to 67c729c Compare November 4, 2020 18:26

sssoleileraaa requested a review from kushaldas November 4, 2020 18:27

emkll reviewed Nov 4, 2020

View reviewed changes

sssoleileraaa force-pushed the add-seen-tables branch from 67c729c to e8bfa5d Compare November 4, 2020 23:37

add seen tables and test migration

05606a6

sssoleileraaa force-pushed the add-seen-tables branch from e8bfa5d to 05606a6 Compare November 5, 2020 18:21

emkll approved these changes Nov 5, 2020

View reviewed changes

emkll merged commit 97cc9f9 into main Nov 5, 2020

emkll deleted the add-seen-tables branch November 5, 2020 18:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add seen tables#1164

Add seen tables#1164
emkll merged 1 commit intomainfrom
add-seen-tables

sssoleileraaa commented Oct 21, 2020

Uh oh!

kushaldas left a comment •

edited

Loading

Uh oh!

sssoleileraaa commented Nov 4, 2020

Uh oh!

sssoleileraaa commented Nov 4, 2020

Uh oh!

emkll left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

emkll left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

sssoleileraaa commented Oct 21, 2020

Description

Test Plan

Uh oh!

kushaldas left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sssoleileraaa commented Nov 4, 2020

Uh oh!

sssoleileraaa commented Nov 4, 2020

Uh oh!

emkll left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

emkll left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kushaldas left a comment •

edited

Loading