Admin - Extend site edit functionality #958
Conversation
![pullrequest[bot]](https://avatars.githubusercontent.com/in/3434?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ This pull request was sent to the PullRequest network.
Check the status or cancel PullRequest code review here.
There was a problem hiding this comment.
PullRequest Breakdown
Reviewable lines of change
+ 640
- 98
45% TSX
42% TypeScript
9% TypeScript (tests)
4% Jest Snapshot (tests)
1% TSX (tests)Type of change
Feature - These changes are adding a new feature or improvement to existing code.
| 1 Message | |
|---|---|
| Due to its size, this pull request will likely have a little longer turnaround time and will probably require multiple passes from our reviewers. |
|
Build succeeded and deployed at https://aqualink-app-958.surge.sh |
There was a problem hiding this comment.
This is looking like a great start, but is missing test coverage. I also have a question inline for your consideration. Other than that, all the changes around falling back to undefined or null or empty string, sometimes using ?? and sometimes using || seem a little inconsistent - what was the reason for those changes?
Reviewed with ❤️ by PullRequest
ericboucher
changed the title
| onGetMetrics(); | ||
| } | ||
| }; | ||
| if (fetchOnEnter) document.addEventListener('keydown', keyDownHandler); |
| @@ -296,26 +297,30 @@ export class MonitoringService { | |||
| : baseQuery; | |||
|
|
|||
| const withSiteName = siteName | |||
| ? withSiteId.andWhere('site.name = :siteName', { siteName }) | |||
| ? withSiteId.andWhere('site.name ILIKE :siteName', { | |||
| siteName: `%${siteName}%`, | |||
There was a problem hiding this comment.
Embedding the siteName, adminEmail, etc in % is incorrect as it omits to take into account that they themselves may contain characters with a semantics other than a literal interpretation, such as a backslash, underscore or percentage symbol. One needs to apply escaping to each string before wrapping them in the percentages.
One should define a function to apply escaping of values so that they can be used in a LIKE expression. For example, this will replace siteName with a safe version of it, that can be wrapped in double quotes without issue.
siteName.replaceAll('\\', '\\\\').replaceAll('_', '\\_').replaceAll('%', '\\%'))
FYI, the replace order matters, at least that all the backslashes are escaped first, then the other two can be escaped in either order.
🔺 Bug (Critical)
Adds a new column
contact_informationto site tableAdds the ability to edit these extra fields: