Fix Number fields store as text

[BibianaC]

Relates to #1225

[BibianaC]

@chrisarridge The links check is failing because of the Standard.

The failing test is the aggregates test. It’s the same issue as the geolocation one. I’ve made some changes in the aggregates file. The test is passing locally for me, but it’s still failing in GitHub.

[chrisarridge]

@chrisarridge The links check is failing because of the Standard.

The failing test is the aggregates test. It’s the same issue as the geolocation one. I’ve made some changes in the aggregates file. The test is passing locally for me, but it’s still failing in GitHub.

The aggregates test fails for me locally as well. Interestingly, if I drop both the commits that change aggregates_expected.json then the test passes absolutely fine. Would you expect the aggregates to change for your change in this case? I'm curious about this given that we are also having issues with the geolocation work.

[BibianaC]

@chrisarridge the process is:

• I make changes.
• Test_api fails.
• I uncomment the commented code.
• Run tests.
• Test Api fails.
• Comment back the code and change the expected aggregates file.
• Run tests.
• Everything is ok.

[coveralls]

Coverage Report for CI Build 26572609936

Coverage decreased (-0.03%) to 72.329%

Details

• Coverage decreased (-0.03%) from the base build.
• Patch coverage: No coverable lines changed in this PR.
• 13 coverage regressions across 1 file.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

13 previously-covered lines in 1 file lost coverage.

File	Lines Losing Coverage	Coverage
grantnav/frontend/funders_search_view.py	13	80.47%

---

Coverage Stats

Relevant Lines:	1984
Covered Lines:	1435
Line Coverage:	72.33%
Coverage Strength:	0.72 hits per line

---

💛 - Coveralls

[BibianaC]

@chrisarridge I have added the file to live. Here, the tests are passing, but locally are not.

[chrisarridge]

Could we add something here that checks the latestIncome fields to make sure they match the range? For example,

for hit in r.context["results"]["hits"]["hits"]:
    recipientOrgInfos = hit["_source"].get("additional_data",{}).get("recipientOrgInfos",[])
    for recipientOrg in recipientOrgInfos:
        if "latestIncome" in recipientOrg:
            self.assertLessEqual(int(recipientOrg["latestIncome"]), 500000)
            self.assertGreaterEqual(int(recipientOrg["latestIncome"]), 10000)

[chrisarridge]

In principle such a check could also be added to the income greater than, and the durations extending similarly.

[chrisarridge]

Looks good. I have a suggestion for expanding the numeric search test. Please could you squash the two dataload modification commits, and the three test data update commits to just leave two commits in the PR? I think this might be cleaner and easier to follow in the repo history.

[mariongalley]

Please can you spin up a dev GrantNav so we can test this? Changes to search functionality have historically had a lot of unintended side-effects and I wouldn't want to put this live without functional testing.

[chrisarridge]

Please can you spin up a dev GrantNav so we can test this? Changes to search functionality have historically had a lot of unintended side-effects and I wouldn't want to put this live without functional testing.

Yes @mariongalley we can do this.

[BibianaC]

Thanks @chrisarridge . I have made the changes.