Add send_state column to updates.

[dracos]

This adds a sending state column to the comment table that the daemon can use for finding what updates still need processing. The schema change sets the default to 'processed' then changes it to 'unprocessed' so that all existing entries are set as processed.

This also means we no longer need workaround such as sticking -1 in the external_id column to stop an unsendable comment being processed, we can set the send_state to processed or skipped instead.

The daemon will fetch all unprocessed confirmed updates on sent reports. Any that lack an external_id or weren't sent via the Open311 send method will be marked as processed. Any that the cobrand says should be skipped will be marked as skipped. Any remainder will be sent, marked as sent if successful, left as unprocessed if not (and the send_fail_timestamp backoff should work as normal).

Before:

bci=# explain analyze SELECT me.* FROM "comment" "me"
JOIN "problem" "problem" ON "problem"."id" = "me"."problem_id" 
WHERE ( ( ( ( "me"."send_fail_count" = 0 OR "me"."send_fail_timestamp" < current_timestamp - '30 minutes'::interval )
AND regexp_split_to_array(bodies_str, ',') && ARRAY[...] )
AND "me"."external_id" IS NULL
AND ( "me"."extra" IS NULL OR "me"."extra" NOT LIKE '%cobrand_skipped_sending%' )
AND "me"."state" = 'confirmed'
AND "me"."whensent" IS NULL
AND "problem"."external_id" IS NOT NULL
AND "problem"."send_method_used" LIKE '%Open311%'
AND "problem"."whensent" IS NOT NULL ) ) LIMIT 1 ;
                                                                                                                                            QUERY PLAN

--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Limit  (cost=0.43..25.60 rows=1 width=768) (actual time=16435.834..16435.836 rows=0 loops=1)
   ->  Nested Loop  (cost=0.43..1721053.65 rows=68374 width=768) (actual time=16435.832..16435.834 rows=0 loops=1)
         ->  Seq Scan on problem  (cost=0.00..553852.70 rows=219810 width=4) (actual time=0.016..4779.016 rows=1269011 loops=1)
               Filter: ((external_id IS NOT NULL) AND (whensent IS NOT NULL) AND (send_method_used ~~ '%Open311%'::text) AND (regexp_split_to_array(bodies_str, ','::text) && '{...}'::text[]))
               Rows Removed by Filter: 2154465
         ->  Index Scan using comment_problem_id_idx on comment me  (cost=0.43..5.30 rows=1 width=768) (actual time=0.009..0.009 rows=0 loops=1269011)
               Index Cond: (problem_id = problem.id)
               Filter: ((external_id IS NULL) AND (whensent IS NULL) AND ((extra IS NULL) OR (extra !~~ '%cobrand_skipped_sending%'::text)) AND (state = 'confirmed'::text) AND ((send_fail_count = 0) OR (send_fail_timestamp < (CURRENT_TIMESTAMP - '00:30:00'::interval))))
               Rows Removed by Filter: 4
 Planning Time: 0.507 ms
 Execution Time: 16435.873 ms

After:

bci=# explain analyze SELECT me.* FROM "comment" "me"
JOIN "problem" "problem" ON "problem"."id" = "me"."problem_id" 
WHERE ( ( ( ( "me"."send_fail_count" = 0 OR "me"."send_fail_timestamp" < current_timestamp - '30 minutes'::interval )
AND regexp_split_to_array(bodies_str, ',') && ARRAY[...] )
AND "me"."state" = 'confirmed'
AND me.send_state = 'unprocessed'
AND "problem"."whensent" IS NOT NULL ) ) LIMIT 1 ;
                                                                                                                                            QUERY PLAN
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Limit  (cost=0.99..15.96 rows=1 width=599) (actual time=0.025..0.025 rows=0 loops=1)
   ->  Nested Loop  (cost=0.99..15.96 rows=1 width=599) (actual time=0.024..0.024 rows=0 loops=1)
         ->  Index Scan using comment_state_send_state_idx on comment me  (cost=0.56..7.50 rows=1 width=599) (actual time=0.024..0.024 rows=0 loops=1)
               Index Cond: ((state = 'confirmed'::text) AND (send_state = 'unprocessed'::text))
               Filter: ((send_fail_count = 0) OR (send_fail_timestamp < (now() - '00:30:00'::interval)))
         ->  Index Scan using problem_pkey on problem  (cost=0.43..8.46 rows=1 width=4) (never executed)
               Index Cond: (id = me.problem_id)
               Filter: ((whensent IS NOT NULL) AND (regexp_split_to_array(bodies_str, ','::text) && '{...}'::text[]))
 Planning time: 0.885 ms
 Execution time: 0.130 ms

[codecov]

Codecov Report

Merging #3865 (efd7e9d) into master (f59a032) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head efd7e9d differs from pull request most recent head 5b640e2. Consider uploading reports for the commit 5b640e2 to get more accurate results

@@            Coverage Diff             @@
##           master    #3865      +/-   ##
==========================================
+ Coverage   82.64%   82.66%   +0.01%     
==========================================
  Files         353      353              
  Lines       24296    24298       +2     
  Branches     3678     3679       +1     
==========================================
+ Hits        20080    20086       +6     
+ Misses       3068     3064       -4     
  Partials     1148     1148              

Impacted Files	Coverage Δ
perllib/FixMyStreet/App/Controller/Admin/Triage.pm	88.23% <ø> (ø)
perllib/FixMyStreet/DB/Result/Comment.pm	91.30% <ø> (ø)
perllib/Open311/PostServiceRequestUpdates.pm	96.34% <100.00%> (+0.28%)	⬆️
web/js/front.js	82.25% <0.00%> (-1.62%)	⬇️
web/cobrands/fixmystreet-uk-councils/alloy.js	83.18% <0.00%> (+0.88%)	⬆️
perllib/Open311.pm	93.72% <0.00%> (+1.65%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f59a032...5b640e2. Read the comment docs.

[davea]

Looks good to me! One thing that would be really handy to have is a clear explanation of what each of the send_state values mean, as I know I'll forget the difference between sent and processed at some point in the future.