I had a chat with Marcel today and he mentioned some tech debt due to getting all the amazing new ownership features into production so quickly.
Since I'm returning with fresh eyes, it feels like a good time to note down the bits of the codebase that I'm finding confusing. Then we can go through and fix each one when we get time. I don't think it should require lots of context for new developers to roughly understand what's going on, and making the code more self-explanatory will reduce the risk of bugs and speed up future development.
Here's the list of things to address (WIP - please add anything else you spot):
[ ] It's not clear what 'ownership search' refers to. Is it when we click on a property boundary, or the new function of searching for properties by owner? It would be helpful to distinguish between these functions in the naming
The comments of the 2 backend APIs explain what each refers to but, as can be seen, it's not super clear. Do they need to be separate APIs since they are both querying the same database tables and returning the same data structures? e.g. could we simplify the code and just have an /ownership API with queries parameters for the different filters, like bounds, proprietor, etc. This would be more extensible too.
// Get the geojson polygons of land ownership within a given bounding box area
{ method: "GET", path: "/api/ownership", handler: getLandOwnershipPolygons },
// search the public ownership information
{ method: "GET", path: "/api/search", handler: searchOwnership },
[ ] the naming of Redux actions and state variables is a bit confusing. We have 'highlighted properties' which are part of the original land ownership layer. And now 'selected properties' which are part of the 'related properties' layer. Maybe we can make this more clear and consistent.
Could we even just use one layer and redux state, and highlight properties and related properties the same, so that we don't also hit the issue of duplication of properties and double-highlighting?
[ ] Not strictly a codebase thing, but related: the 'Check for other properties' button was unclear to me, when playing around on the app. I think it should be obvious that this is a check for other properties with the same owner
Acceptance Criteria
The codebase becomes simpler and easier to understand as a new developer, with the above issues addressed.
Description
I had a chat with Marcel today and he mentioned some tech debt due to getting all the amazing new ownership features into production so quickly.
Since I'm returning with fresh eyes, it feels like a good time to note down the bits of the codebase that I'm finding confusing. Then we can go through and fix each one when we get time. I don't think it should require lots of context for new developers to roughly understand what's going on, and making the code more self-explanatory will reduce the risk of bugs and speed up future development.
Here's the list of things to address (WIP - please add anything else you spot):
Acceptance Criteria
The codebase becomes simpler and easier to understand as a new developer, with the above issues addressed.