fablabbcn / smartcitizen-api

The Smart Citizen Engine
https://developer.smartcitizen.me
GNU Affero General Public License v3.0
10 stars 4 forks source link

Delete archived devices which have a null archived_at date. #258

Closed timcowlishaw closed 9 months ago

timcowlishaw commented 9 months ago

Fixes #252. For some reason, we've got some legacy devices which never had their archived_at date set, which are causing the deletion job to crash, meaning that neither they (nor archived devices with subsequent IDs) are deleted. This adds a test for the failure, and fixes so that all archived devices with a null archived_at are deleted. This shouldn't cause a problem in future as only old devices have the null archived_at date.

oscgonfer commented 9 months ago

Hi @timcowlishaw

Thinking of potential future implications of allowing null archived_at dates, I am afraid of what happens to the 24h grace period for a user that "changes its mind".

Would it be maybe be better to add a date to those that don't have it and have the normal workflow go through these devices?

In other words, if those null archived_at dates had a date, would it work normally? And, could we then consider these null bits a fluke?

oscgonfer commented 9 months ago

For now we don't merge this PR, as we have added a timestamp manually. We will come back to this in the following days.

oscgonfer commented 9 months ago

Hi @timcowlishaw

I think this is solved now:

smartcitizen_production=# select id, created_at, workflow_state, archived_at from devices where workflow_state = 'archived';
 id | created_at | workflow_state | archived_at 
----+------------+----------------+-------------
(0 rows)

smartcitizen_production=# select count(id), workflow_state from devices group by workflow_state;
 count | workflow_state 
-------+----------------
  7867 | active
(1 row)

smartcitizen_production=# select id, created_at, workflow_state, archived_at from devices where workflow_state = 'archived';
 id | created_at | workflow_state | archived_at 
----+------------+----------------+-------------
(0 rows)

OK for you to close the PR?

timcowlishaw commented 9 months ago

Aha great stuff. Sure thing, no need for the PR now it's working!

El dv., 6 oct. 2023, 13.53, oscgonfer @.***> va escriure:

Hi @timcowlishaw https://github.com/timcowlishaw

I think this is solved now:

smartcitizen_production=# select id, created_at, workflow_state, archived_at from devices where workflow_state = 'archived'; id | created_at | workflow_state | archived_at ----+------------+----------------+------------- (0 rows)

smartcitizen_production=# select count(id), workflow_state from devices group by workflow_state; count | workflow_state -------+---------------- 7867 | active (1 row)

smartcitizen_production=# select id, created_at, workflow_state, archived_at from devices where workflow_state = 'archived'; id | created_at | workflow_state | archived_at ----+------------+----------------+------------- (0 rows)

OK for you to close the PR?

— Reply to this email directly, view it on GitHub https://github.com/fablabbcn/smartcitizen-api/pull/258#issuecomment-1750512904, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAABBN52YETN7Q4KWFZVDZLX57WLTAVCNFSM6AAAAAA5MM5Q7CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONJQGUYTEOJQGQ . You are receiving this because you were mentioned.Message ID: @.***>