Repository: sec-edgar/sec-edgar

Issue:
Count/params not resetting after `get_urls`
**Describe the bug**
`CompanyFilings` not resetting start parameter after using `get_urls` method.

**To Reproduce**
```python
from secedgar.core.company import CompanyFilings

a = CompanyFilings(cik_lookup='aapl', start_date=datetime(2021,2,28))
print(a.params)
first_urls = a.get_urls() # returns urls
print(a.params)
second_urls = a.get_urls() # Will not return same urls as first_urls
```

**Expected behavior**
Should reset start parameter after while loop.


Discussion:

Patch:
diff --git a/docs/source/whatsnew/v0.4.0.rst b/docs/source/whatsnew/v0.4.0.rst
index 52354c6..af6620b 100644
--- a/docs/source/whatsnew/v0.4.0.rst
+++ b/docs/source/whatsnew/v0.4.0.rst
@@ -16,6 +16,7 @@ Many breaking changes, as this update does a lot of restructuring.
 - Fix use of ``user_agent`` by ``NetworkClient`` and require its use by all filings classes/functions
 - Fix issue where daily CLI command would not work due to ``date_cleanup`` returning ``datetime.datetime`` instead of ``datetime.date``
 - Fix issue where ``CompanyFilings`` includes links that are not exact matches to ``filing_type``.
+- Fix issue where ``get_urls`` does not reset ``params`` after getting company filings URLs.
 - Add NSAR filing types. Thanks to mjkoo91!
 - Get rid of caching behavior that was causing issues when downloading filings.
 - Simplify ``ComboFilings`` logic for getting quarterly and daily dates.
diff --git a/secedgar/core/company.py b/secedgar/core/company.py
index c4fa20e..b4480c4 100644
--- a/secedgar/core/company.py
+++ b/secedgar/core/company.py
@@ -107,12 +107,10 @@ class CompanyFilings(AbstractFiling):
                  ownership="include",
                  match_format="ALL",
                  **kwargs):
-        # Leave params before other setters
         self._params = {
             "action": "getcompany",
             "output": "xml",
-            "owner": ownership,
-            "start": 0,
+            "start": 0
         }
         self.start_date = start_date
         self.end_date = end_date
@@ -123,6 +121,7 @@ class CompanyFilings(AbstractFiling):
         self._client = client or NetworkClient(user_agent=user_agent, **kwargs)
         # make CIKLookup object for users if not given
         self.cik_lookup = cik_lookup
+        self.ownership = ownership
 
     @property
     def path(self):
@@ -132,6 +131,17 @@ class CompanyFilings(AbstractFiling):
     @property
     def params(self):
         """:obj:`dict`: Parameters to include in requests."""
+        if self.start_date:
+            self._params["datea"] = sanitize_date(self.start_date)
+        else:
+            self._params.pop("datea", None)  # if no start date, make sure it isn't in params
+
+        if self.end_date:
+            self._params["dateb"] = sanitize_date(self.end_date)
+        else:
+            self._params.pop("dateb", None)  # if no end date, make sure it isn't in params
+
+        self._params["ownership"] = self.ownership
         return self._params
 
     @property
@@ -151,18 +161,14 @@ class CompanyFilings(AbstractFiling):
 
     @match_format.setter
     def match_format(self, val):
-        if val in ["EXACT", "AMEND", "ALL"]:
-            self._match_format = val
-        else:
-            raise ValueError("Format must be one of EXACT,AMEND,ALL")
+        if val not in ("EXACT", "AMEND", "ALL"):
+            raise ValueError("Format must be 'EXACT', 'AMEND', or 'ALL'.")
+        self._match_format = val
 
     @start_date.setter
     def start_date(self, val):
-        if val is not None:
-            self._params["datea"] = sanitize_date(val)
-            self._start_date = val
-        else:
-            self._start_date = None
+        sanitize_date(val)  # make sure start date is valid
+        self._start_date = val
 
     @property
     def end_date(self):
@@ -171,7 +177,7 @@ class CompanyFilings(AbstractFiling):
 
     @end_date.setter
     def end_date(self, val):
-        self._params["dateb"] = sanitize_date(val)
+        sanitize_date(val)  # make sure end date is valid
         self._end_date = val
 
     @property
@@ -184,7 +190,9 @@ class CompanyFilings(AbstractFiling):
         if isinstance(filing_type, FilingType):
             self._params["type"] = filing_type.value
         elif filing_type is not None:
-            raise FilingTypeError
+            raise FilingTypeError(
+                f"filing_type must be of type FilingType. Given {type(filing_type)}."
+            )
         self._filing_type = filing_type
 
     @property
@@ -215,6 +223,17 @@ class CompanyFilings(AbstractFiling):
             val = CIKLookup(val, client=self.client)
         self._cik_lookup = val
 
+    @property
+    def ownership(self):
+        """str: Whether or not to include ownership in search."""
+        return self._ownership
+
+    @ownership.setter
+    def ownership(self, val):
+        if val not in ("include", "exclude"):
+            raise ValueError(f"Ownership must be 'include' or 'exclude'. Given {val}.")
+        self._ownership = val
+
     def get_urls(self, **kwargs):
         """Get urls for all CIKs given to Filing object.
 


Test patch:
diff --git a/secedgar/tests/core/test_company.py b/secedgar/tests/core/test_company.py
index 238cfe9..744e8e7 100644
--- a/secedgar/tests/core/test_company.py
+++ b/secedgar/tests/core/test_company.py
@@ -72,6 +72,24 @@ def mock_single_cik_filing_limited_responses(monkeypatch):
 
 
 class TestCompanyFilings:
+    valid_dates = [
+        datetime.datetime(2020, 1, 1),
+        datetime.datetime(2020, 2, 1),
+        datetime.datetime(2020, 3, 1),
+        datetime.datetime(2020, 4, 1),
+        datetime.datetime(2020, 5, 1),
+        "20200101",
+        20200101,
+        None
+    ]
+    bad_dates = [
+        1,
+        2020010101,
+        "2020010101",
+        "2020",
+        "0102"
+    ]
+
     class TestCompanyFilingsClient:
 
         def test_user_agent_client_none(self):
@@ -114,13 +132,7 @@ class TestCompanyFilings:
                                 count=count)
         assert filing.count == count
 
-    @pytest.mark.parametrize("start_date", [
-        datetime.datetime(2020, 1, 1),
-        datetime.datetime(2020, 2, 1),
-        datetime.datetime(2020, 3, 1),
-        datetime.datetime(2020, 4, 1),
-        datetime.datetime(2020, 5, 1), "20200101", 20200101, None
-    ])
+    @pytest.mark.parametrize("start_date", valid_dates)
     def test_good_start_date_setter_on_init(self, start_date, mock_user_agent):
         filing = CompanyFilings(
             cik_lookup="aapl",
@@ -129,8 +141,7 @@ class TestCompanyFilings:
             user_agent=mock_user_agent)
         assert filing.start_date == start_date
 
-    @pytest.mark.parametrize("bad_start_date",
-                             [1, 2020010101, "2020010101", "2020", "0102"])
+    @pytest.mark.parametrize("bad_start_date", bad_dates)
     def test_bad_start_date_setter_on_init(self, mock_user_agent, bad_start_date):
         with pytest.raises(TypeError):
             CompanyFilings(user_agent=mock_user_agent,
@@ -138,6 +149,23 @@ class TestCompanyFilings:
                            filing_type=FilingType.FILING_10Q,
                            start_date=bad_start_date)
 
+    @pytest.mark.parametrize("end_date", valid_dates)
+    def test_good_end_date_setter_on_init(self, end_date, mock_user_agent):
+        filing = CompanyFilings(
+            cik_lookup="aapl",
+            filing_type=FilingType.FILING_10Q,
+            end_date=end_date,
+            user_agent=mock_user_agent)
+        assert filing.end_date == end_date
+
+    @pytest.mark.parametrize("bad_end_date", bad_dates)
+    def test_bad_end_date_setter_on_init(self, mock_user_agent, bad_end_date):
+        with pytest.raises(TypeError):
+            CompanyFilings(user_agent=mock_user_agent,
+                           cik_lookup="aapl",
+                           filing_type=FilingType.FILING_10Q,
+                           end_date=bad_end_date)
+
     @pytest.mark.parametrize("count,expected_error", [(-1, ValueError),
                                                       (0, ValueError),
                                                       (0.0, TypeError),
@@ -325,7 +353,6 @@ class TestCompanyFilings:
         my_filings.save(tmp_data_directory)
 
     def test__filter_filing_links(self, mock_user_agent, mock_single_cik_filing):
-        # data =
         f = CompanyFilings(cik_lookup="aapl",
                            filing_type=FilingType.FILING_10Q,
                            user_agent=mock_user_agent)
@@ -333,3 +360,85 @@ class TestCompanyFilings:
         links = f._filter_filing_links(data)
         assert len(links) == 10
         assert all(["BAD_LINK" not in link for link in links])
+
+    def test_same_urls_fetched(self, mock_user_agent, mock_single_cik_filing):
+        # mock_single_filing_cik has more than 10 URLs
+        # using count = 5 should help test whether the same URLs
+        # are fetched each time
+        f = CompanyFilings(cik_lookup="aapl",
+                           filing_type=FilingType.FILING_10Q,
+                           user_agent=mock_user_agent,
+                           count=5)
+        first_urls = f.get_urls()
+        second_urls = f.get_urls()
+        assert all(f == s for f, s in zip(first_urls, second_urls))
+
+    @pytest.mark.parametrize(
+        "bad_ownership",
+        [
+            "notright",
+            "_exclude",
+            "_include",
+            "notvalid",
+            1,
+            True,
+            False
+        ]
+    )
+    def test_ownership(self, bad_ownership, mock_user_agent):
+        with pytest.raises(ValueError):
+            CompanyFilings(
+                cik_lookup="aapl",
+                filing_type=FilingType.FILING_10Q,
+                user_agent=mock_user_agent,
+                ownership=bad_ownership
+            )
+
+    def test_good_ownership(self, mock_user_agent):
+        shared_params = dict(
+            cik_lookup="aapl",
+            filing_type=FilingType.FILING_10Q,
+            user_agent=mock_user_agent,
+        )
+        f_include = CompanyFilings(
+            **shared_params,
+            ownership="include"
+        )
+        f_exclude = CompanyFilings(
+            **shared_params,
+            ownership="exclude"
+        )
+        assert f_include.ownership == "include"
+        assert f_exclude.ownership == "exclude"
+
+        # Change ownership type
+        f_include.ownership = "exclude"
+        assert f_include.ownership == "exclude"
+
+    def test_start_date_change_to_none(self, mock_user_agent):
+        start_date = datetime.date(2020, 1, 1)
+        f = CompanyFilings(
+            cik_lookup="aapl",
+            filing_type=FilingType.FILING_10Q,
+            user_agent=mock_user_agent,
+            start_date=start_date
+        )
+        assert f.start_date == start_date
+        assert f.params["datea"] == "20200101"
+        f.start_date = None
+        assert f.start_date is None
+        assert "datea" not in f.params
+
+    def test_end_date_change_to_none(self, mock_user_agent):
+        end_date = datetime.date(2020, 1, 1)
+        f = CompanyFilings(
+            cik_lookup="aapl",
+            filing_type=FilingType.FILING_10Q,
+            user_agent=mock_user_agent,
+            end_date=end_date
+        )
+        assert f.end_date == end_date
+        assert f.params["dateb"] == "20200101"
+        f.end_date = None
+        assert f.end_date is None
+        assert "dateb" not in f.params