richardxia
commented
1 year ago Sorry for the late review, I was away this weekend. This looks great! This is some heavy duty typescript wizardry. I'm excited that this is near the finish line.
No problem, and thanks as always for reviewing these PRs!
Refs #1175.
I feel like I'm finally getting close to the end here. This should be the last subcomponent of the Edit Page that needs type annotations, and I think the bulk of the remaining missing type annotations are all on the top-level OrganizationEditPage itself.
This PR is a bit bigger than the last several that I've submitted. There were a couple refactoring changes I made before actually adding type annotations to the code:
countryfield from theAddresstype. We had actually removed this field from the API side while working on the Multiple Addresses project in https://github.com/ShelterTechSF/askdarcel-api/pull/586, since it was confusing to users on the edit page, and we left some vestiges of it on the frontend just to decouple the merging of the PRs. This commit finally removes those vestiges from the frontend codeAddresstype by making them nullable. This makes it match the API backend's DB schema874ae97669165ca34a3fc206cfc03f561fad8614 is finally the bulk of the changes, which are to add type annotations to the EditAddress component and all components related to it. I think probably the most notable thing here is that I created a new
InternalAddresstype, matching the convention for other types I've defined that are derived from the common ones but specialized to the Edit Page. Here, the main differences are that these reflect only the fields that are visible on the Edit Page, as we don't even display a number of the fields available in the DB table, such as address lines 3 and 4 and the attention field.There are other little gotchas related to the type system that I had to deal with, but I believe any of the really tricky ones should have comments above them.
One more thing that I feel I should provide a bit of context for is the notion of an "address handle", since some of the code in the diff is related to that. This is something I added during the Multiple Addresses work, and it's supposed to be a field on Services which hold the integer index of an Address defined on the Resource. We need this on the Edit Page in order to simplify the logic on the Edit Page.
Normally, the API returns copies of an Address on the Resource and on the Services, meaning that address information is duplicated between Resources and Services and even across different Services at the same address. On the read/listing pages, this is fine and even a bit convenient, but on the edit page, this makes it extremely problematic to keep track of edits to addresses, since the edits may be made to one address but not the other copies. The way we were originally handling this on the Edit Page was to compute diffs, which was somewhat tractable to do when we could only have a single Address per Resource, and Services could only be at that same address.
To make this work with multiple addresses, right after the Edit Page loads and when the API response with the initial page data comes back from the API, we transform the data such that we remove the copies of the Addresses on all the Services, and we replace them with these address handles. These are like "pointers" in languages that have pointers. By removing all the Addresses on the Services, we ensure that there is only one copy of any particular Address, and it is stored on the Resource. (Actually, it's more complicated than that because we always have two copies of everything on the Edit Page: the original, as it came from the API, and a modified version) This way, when we edit an Address, we only edit a single copy of it, and when we need to determine whether we need to send an update request to the API, we just need to check that one Address.
I hope that wasn't too long-winded of an explanation. I think this is documented in the comments on the Edit Page somewhere, but I wanted to summarize all that in case you were wondering just what an "address handle" is.