Update annotation project update endpoint. Update status DB trigger #5461
Conversation
There was a problem hiding this comment.
Let me know what you think about the proposed changes -- I'm not sure what the blast radius is for campaign project count stuff (I think it will support future Groundwork campaign stuff as well?) but if it's large I think the tests should be shored up a little bit
| @@ -302,6 +302,16 @@ class AnnotationProjectDaoSpec | |||
| "Readiness was updated" | |||
| ) | |||
|
|
|||
| assert( | |||
| afterUpdate.campaignId == annotationProjectUpdate.campaignId, | |||
There was a problem hiding this comment.
I think this technically isn't testing anything, since the test would still pass if the logic to update the campaign ID were removed.. The generator for AnnotationProject.Creates doesn't include a campaign id, so both of them start with None and should end with None.
There was a problem hiding this comment.
This is a good call. I will add the campaign portion.
| ), | ||
| user | ||
| ) | ||
| _ <- AnnotationProjectDao.update( |
There was a problem hiding this comment.
I think to show the update it would be good to get the campaign before updating the projects and have an expectation that it should have two waiting projects.
Also, since you're deleting this project anyway and it doesn't affect the expectation, is it correct that the test is the same without this query?
There was a problem hiding this comment.
Yea, so I intended to pluck out campaign status in multiple spots before and after the annotation projects' updates, and somehow forgot to do so in the end, this is why you see multiple updates to annotation projects.
This part of logic (in terms of listening to project status updates) has been tested in ER prod for some months now, and it has been working fine. The postgres function updated in PR is just to let it listen to delete actions as well.
I will add more status checks in the next commit.
|
@jisantuc I'd like to do a slack call with you, since I don't think I fully understood what you meant by the blast radius for the campaign project count stuffs. |
|
Update: we had a successful call on slack, we decided to:
|
Overview
This PR:
campaign_idand thecaptured_atfieldannotation_projectsso that the associatedcampaign'sproject_statusesis in-sync. We used to only listen toUPDATE OF status, now we listen toDELETEas wellChecklist
Swagger specification updatedNew tables and queries have appropriate indices addedAny content changes are properly templated usingBUILDCONFIG.APP_NAMEAny new endpoints have scope validation and are included in the integration test csvTesting Instructions
Closes https://github.com/azavea/earthrise/issues/135