diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a8e7008fa..073c15307c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Fixed - Fixed a potential deadlock when using a small connection pool and `store`-ing queries - AutoFlow can now be run in a docker container with non-default user. [#5574](https://github.com/Flowminder/FlowKit/issues/5574) +- Passing an empty list of events tables when creating a query now raises `ValueError: Empty tables list.` instead of a `MissingDateError`. [#436](https://github.com/Flowminder/FlowKit/issues/436) ### Removed diff --git a/flowmachine/flowmachine/features/utilities/events_tables_union.py b/flowmachine/flowmachine/features/utilities/events_tables_union.py index 57bb6671bc..960b33bd35 100644 --- a/flowmachine/flowmachine/features/utilities/events_tables_union.py +++ b/flowmachine/flowmachine/features/utilities/events_tables_union.py @@ -84,8 +84,16 @@ def column_names(self) -> List[str]: def _parse_tables(self, tables): if tables is None: return [f"events.{t}" for t in get_db().subscriber_tables] - elif isinstance(tables, str): + elif isinstance(tables, str) and len(tables) > 0: return [tables] + elif isinstance(tables, str): + raise ValueError("Empty table name.") + elif not isinstance(tables, list) or not all( + [isinstance(tbl, str) for tbl in tables] + ): + raise ValueError("Tables must be a string or list of strings.") + elif len(tables) == 0: + raise ValueError("Empty tables list.") else: return tables diff --git a/flowmachine/tests/test_events_table_union.py b/flowmachine/tests/test_events_table_union.py index 5c9e528c97..2680a12279 100644 --- a/flowmachine/tests/test_events_table_union.py +++ b/flowmachine/tests/test_events_table_union.py @@ -62,6 +62,29 @@ def test_get_only_sms(get_length): assert get_length(etu) == 1246 +@pytest.mark.parametrize( + "arg,error_type,error_message", + [ + ("", ValueError, "Empty table name."), + (0, ValueError, "Tables must be a string or list of strings."), + ([0, "a"], ValueError, "Tables must be a string or list of strings."), + ([], ValueError, "Empty tables list."), + ], +) +def test_bad_table_arguments(arg, error_type, error_message): + """ + Test that an appropriate error is raised for bad tables arguments. + """ + + with pytest.raises(expected_exception=error_type, match=error_message): + EventsTablesUnion( + "2016-01-01", + "2016-01-02", + columns=["msisdn"], + tables=arg, + ) + + def test_get_list_of_tables(get_length): """ Test that we can get only sms