Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PROD] Field report "affected" values in different database cells #1561

Open
szabozoltan69 opened this issue Oct 21, 2022 · 8 comments
Open
Assignees

Comments

@szabozoltan69
Copy link
Contributor

szabozoltan69 commented Oct 21, 2022

Issue

Recently it is impossible to know, which of these figures was sent last time in a Field Report – and we should consider only one:
num_affected
gov_num_affected
other_num_affected

num_potentially_affected
gov_num_potentially_affected
other_num_potentially_affected

So we introduce a new field on backend only, to document: which was sent last time (which is considered valid currently).
When the data are sent to frontend client, we should send zero for the other field values.

It has also relevance when GO sends Field Report data to ERP.
Thank you, @mmusori for the catch!

@szabozoltan69 szabozoltan69 self-assigned this Oct 21, 2022
@szabozoltan69
Copy link
Contributor Author

szabozoltan69 commented Oct 21, 2022

When there are multiple "_affected" values, recently the frontend solves the conflict (on Edit page) according to "strength": https://github.com/IFRCGo/go-frontend/blob/develop/src/root/views/FieldReportForm/common.tsx#L841-L846

If RedCross-figure is provided, we use that. If Government-figure is given, we use that. Otherwise we use the Other-figure to be edited.

@szabozoltan69
Copy link
Contributor Author

szabozoltan69 commented Oct 21, 2022

Plan:

    class RecentAffected(models.IntegerChoices):
        RCRC = 1, _('Red Cross / Red Crescent')
        GOVERNMENT = 2, _('Government')
        OTHER = 3, _('Other')
        RCRC_POTENTIALLY = 4, _('Red Cross / Red Crescent, potentially')
        GOVERNMENT_POTENTIALLY = 5, _('Government, potentially')
        OTHER_POTENTIALLY = 6, _('Other, potentially')

@szabozoltan69
Copy link
Contributor Author

szabozoltan69 commented Nov 2, 2022

The recent solution is:

  • on frontend side we can not see any new features, every tricks are on backend.
  • on backend admin side the situation is the following recently:
    • On FR admin page there are some "figure triplets" which we keep in the database in different fields. E.g. injured. We keep RCRC, GOV and other figure, but the showing (on frontend) decision is: if we have RCRC, we show that. If GOV, we show that. Otherwise OTHER (if we have it).
    • in early warnings there are other 3 fields for these, "potentially...".
    • For Affected figures we do not follow the previously mentioned method (RCRC first if have, GOV if have, else OTHER), but introduced a new field (recent_affected) which keeps, which of the 6 possible fields (RCRC, GOV, OTHER, RCRC potentially, GOV potentially, OTHER potentially) is to be used. For this there is a separate dropdown selector on FR Admin page (Recent source of affected people). This is set automatically, when data arrives from frontend client.
    • We show this Affected figure on the frontend Emergency page also, getting the data from the FR fields. (For Emergencies there are no such fields.)
    • From the API we send only the relevant "Affected" figure to the frontend client, so it can use the old method to show it.
  • For ERP we send only the recent affected figure. That was the main motivation for all these: how to decide, which one to send?

@justinginnetti proposed, that (besides the three existing radio buttons) there should be a link also to the Global Crisis Data Bank
@mmusori @LukeCaley @tovari @batpad @thenav please review it.

(The user journey is: which is written the last time on frontend, it can be seen on every frontend page. On backend Admin page it is possible to change those ones, which are NOT the displayed ones.)

@batpad
Copy link
Collaborator

batpad commented Nov 2, 2022

So, I'd like to take a bit of a step back and re-think this a little bit. The different data points we collect for field reports and the multiple sources for them is a feature that grew out over time, and I believe is not modelled correctly in the database, which is causing a lot of these issues.

Currently, we specify the different data points and their sources as separate fields on the FieldReport model, like so: https://github.com/IFRCGo/go-api/blob/develop/api/models.py#L1251 . This was fine when we had only a few fields - once we started adding multiple sources and more fields, we ideally should have moved to a different data model. But, it's never too late. What I would propose:

  • Create a new model, say FieldReportFigure or so, with something like:
    • field_report - FK to Field Report
    • key - This would be like num_affected, num_missing, num_dead, etc.
    • source - This would be IFRC, Gov, Other, etc.
    • value - The value - 250, 300, etc.
    • updated_timestamp - the timestamp this value was updated at.

I believe this will make things much cleaner - it will allow us to be flexible in adding new data points and new sources, and also allow us to manage updated timestamps of each value individually. Once we have this, we can be flexible in where we show what data on the frontend based on source and updated timestamp.

Unfortunately, this will be a lot of work:

  • Migrating all current data to the new table
  • Changing a lot of forms code and possibly the frontend
  • Either changing the API to return data in a new structure or doing a lot of awkward work to keep backwards compatibility on the API.

@thenav56 @frozenhelium @szabozoltan69 how does this sound? If this feels like the correct approach to the problem (and a few related awkward things about these values + sources), we can at least scope it out and decide if that's something we want to do and / or if we need to pursue an intermediate solution if there is an urgent requirement.

@szabozoltan69
Copy link
Contributor Author

szabozoltan69 commented Nov 3, 2022

Really a great idea! The correct approach is always beautiful. I'm looking forward to implement it.
The good part: we can keep the old and new system concurrently on backend, until we decide that "the new system is fine" (or longer, for backward compatibility). Of course, the frontend must be changed also.
A similar FK relationship: country and its districts. #1577

Do I assume well that the "latest" input will be relevant for other num_... fields also?
So no more priority towards RCRC source on frontend? Because on frontend this is the priority now:
https://github.com/IFRCGo/go-frontend/blob/develop/src/root/views/FieldReportForm/common.tsx#L841-L846

@szabozoltan69
Copy link
Contributor Author

Is this list complete?

    class FigureType(models.IntegerChoices):
        UNKNOWN = 0, _('Unknown')
        AFFECTED = 1, _('Affected')
        P_AFFECTED = 2, _('Potentially affected')
        ASSISTED = 3, _('Assisted')
        DEAD = 4, _('Dead')
        DISPLACED = 5, _('Displaced')
        HIGHEST_RISK = 6, _('Highest risk')
        INJURED = 7, _('Injured')
        MISSING = 8, _('Missing')

    class FigureSource(models.IntegerChoices):
        RC = 1, _('Red Cross / Red Crescent')
        GOV = 2, _('Government')
        GCDB = 3, _('Global Crisis Data Bank')
        UN = 4, _('UN')
        OTHER = 5, _('Other')

@szabozoltan69
Copy link
Contributor Author

@batpad There are a lot of other "named integer" fields in field report; should we pull them out too? ERU-s, response figures...
https://github.com/IFRCGo/go-api/blob/develop/api/models.py#L1277-L1326

@batpad
Copy link
Collaborator

batpad commented Nov 8, 2022

I would not worry about the ERU figures for now -- I think they could go into a separate FieldReportERUFigure table or so in the future - but we do not have the problem of multiple sources here (there is no separate "source" for the ERU figures") so I think it's okay to keep in the field report model for now.

What worries me a little bit is the fields that have sources that are not integer fields -- for example "affected_pop_centers" which is a string field -- I'm not sure how best to handle this -- I think we can either make the value field a CharField but then do validation checks that the values other than affected_pop_centers is numerical. Not super sure what the best approach here would be -- or to just use the new structure for Integer Fields, but then still have the CharFields behaving the old way, which I don't like so much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants