age error priority issue

colouring-cities / colouring-britain

Developed out of the Colouring London prototype. Collecting data on Britain's buildings and testing new core features

https://colouringbritain.org/

GNU General Public License v3.0

10 stars 2 forks source link

age error priority issue #373

Open polly64 opened 1 month ago

polly64 commented 1 month ago

Hi @mdsimpson42 all the construction dates are under completion dates can you address today thanks,

mdsimpson42 commented 1 month ago

The building age fields are working as intended. I repurposed the existing data fields, so:

'Year of construction of main building' has become 'Year of construction completed (best estimate)'
'Earliest possible start year' is now the 'Year construction started (best estimate)'

The order and wording of the new fields are what we agreed to on our Zoom call on Tuesday.

If this is not the intended behaviour, I need to know what is wrong and how it should work in order to fix it.

polly64 commented 1 month ago

@mdsimpson42 .

[x] 'the year of construction of main building' needs to be connected to 'Year construction started (best estimate)' as these both refer to the start dates
[x] 'the year of construction completed' should be blank and should also read 'Year construction completed' ie omit 'of'

mdsimpson42 commented 1 month ago

Are we 100% certain about this? This seemed like the most logical way to handle the existing data that we already have in the database.

If I swap the fields over, then the dates won't make sense with the data that we already have. i.e. in this example, the start date would be AFTER the end date:

The other option is to only keep the old 'Year of construction of main building' and rename it to 'Year construction started (best estimate)', then add a NEW field called 'Year construction completed', but then we're effectively throwing away the 'Earliest/latest possible start year'.

mdsimpson42 commented 1 month ago

These changes will need to be made in the Core platform, tested, merged and then pulled from Core into Britain.

We merged them without code review or testing to get them deployed quickly, which goes against best practice and can lead to errors. In future, I will insist that changes like this get deployed to Staging first, so they can be tested before we merge them into the code and deploy them to Production. It will take longer, but it will reduce the number of issues like this that arise.

polly64 commented 1 month ago

@mdsimpson42 can you just reverse what was done and then just add 'completion' date as the we need to correct asap, thanks. Happy to test on staging.