search

renalreg / ukrr_models

SQL Alchemy models for the UKRR Database. Also exports to RDA.
MIT License
0 stars 0 forks source link

Update cod_edta1 in the DB to varchar #18

Open andyatterton opened 5 months ago

andyatterton commented 5 months ago

TODO: These should be updated to String in the DB. cod_edta1 = Column(Integer)

This comment was left in the code and now that the actions are working properly this is not allowed because the TODO doesn't link to an issue or a Jira ticket, hence the issue. It would be useful if @George-D-S could clarify why this should be done and what the thinking is. Does this also extend to cod_edta2 = Column(Integer).

George-D-S commented 5 months ago

Any numeric value which is a code and not a quantity (EDTA, UKRR Modality, SNOMED, etc.) should be stored as a string not a number.

Historically the UKRR didn't do this and indeed took some odd advantages of this such as "Modality + 40" = "Transfer in on Modality", and incrementing the "Not Known" EDTA code as a counter for how many quarters a unit had failed to supply a value. These assumptions then had to be worked round as the code lists evolved. We don't want to do this again.

I thought this was a UKRDC thing so I was going to say "can't make the change until CUPID" but as it's a UKRR thing it's more a "there are a lot more urgent tickets thing".

George-D-S commented 5 months ago

Oops, not sure I meant to close this.

andyatterton commented 5 months ago

I thought this was a UKRDC thing so I was going to say "can't make the change until CUPID" but as it's a UKRR thing it's more a "there are a lot more urgent tickets thing".

This is just to meet the demands of the GitHub action which will not allow TODO: something in the code any more. You need to have TODO: [GIT-18] or something similar that points to an issue. This way we don't forget things like this (this comment is two years old). In my head, this would be a very quick fix, but no doubt there are a bunch of reasons why this would not be the case.