Fix missing data from CSV download

[echaidemenos]

resolves #879

Basically moves CSV creation to the backend, fixing the issue of missing date when selecting all dates.
The issue was caused because the backend limits time-series data to 3 months if no other limit is specified.

[pullrequest]

✅ This pull request was sent to the PullRequest network.

---

Check the status or cancel PullRequest code review here.

[pullrequest]

PullRequest Breakdown

Reviewable lines of change

+ 319
- 206

63% TypeScript
33% TSX
3% TypeScript (tests)
1% JSON (tests)
1% JSON

Generated lines of change

+ 15
- 5

Type of change

Fix - These changes are likely to be fixing a bug or issue.

[github-actions]

Build succeeded and deployed at https://aqualink-app-899.surge.sh
(hash ff2c9f9 deployed at 2023-07-24T16:14:03)

[pullrequest]

This is directionally correct, but I have a few concerns left inline for your consideration, some I am aware were issues before, but in touched code or very close to touched code, so I have brought them up to be on the safe side. Other than those issues, a little more test coverage would be great to see.

Graham C

---

Reviewed with ❤️ by PullRequest

[pullrequest]

This is looking much better. We still have an issue with unbounded work due to no checks on the to and from date, and I've noted a few more things inline for your consideration. Bounded work is important to make sure that the database is not overloaded and that the response size is small enough to not cause issues.

There's a great talk https://www.youtube.com/watch?v=Fup5vHEvU50 that covers things like bounded work, which matters for availability, which is one of the three tiers for good security: confidentiality, integrity, availability - the so called CIA triad.

Graham C

---

Reviewed with ❤️ by PullRequest

[pullrequest]

This encoding is wrapping the wrong thing. The while filename needs to be in there, not just the variable part, as per the code snippet from the previous review.

---

const filename = `${surveyPointDataRangeDto.source}_example.csv`;
res.set({
  'Content-Disposition': `attachment; filename=${encodeURIComponent(filename)}`,
});

🔸 Bug (Important)

Graham C

[echaidemenos]

it is the same:

res.set({
      'Content-Disposition': `attachment; filename=${encodeURIComponent(
        `${surveyPointDataRangeDto.source}_example.csv`,
      )}`,
    });

encodeURIComponent wraps

`${surveyPointDataRangeDto.source}_example.csv`

[pullrequest]

The output is the same, but the code no longer violates separation of concerns. We should not assume that encoding _example.csv doesn't change it.

Graham C

[echaidemenos]

Yes, the code does not assume that _example.csv doesn't change. It is wrapped by the encodeURIComponent call. What i meant is the same was that i don't declare

const filename = `${surveyPointDataRangeDto.source}_example.csv`;

but i just call encodeURIComponent with ${surveyPointDataRangeDto.source}_example.csv as argument

[pullrequest]

Because you are assuming that _example.csv is the output of encodeURIComponent('_example.csv'), which breaks separation of concerns.

Graham C

[pullrequest]

Great work. Thanks for being so patient with my feedback. 🚀

Graham C

---

Reviewed with ❤️ by PullRequest

[pullrequest]

PullRequest reviewed the updates made to #899 up until the latest commit (ff2c9f9). No further issues were found.

Reviewed by:

Graham C