[PROD] Ensure Appeals always have a country associated

[batpad]

Yesterday, we had an error on production after an Appeal was ingested without having a Country attached. We need to make sure this cannot happen again.

If it is legitimate / possible that Appeals can be without a Country, then we should make sure the frontend does not fail if an Appeal is missing a Country.

However, I think in all likelihood, we just want to ensure on the backend that this cannot happen that an Appeal is created without a Country.

We should probably:

• Remove the null=True for country on the Appeal model, so that the database does not allow null values: https://github.com/IFRCGo/go-api/blob/develop/api/models.py#L895 - this will require checking all existing appeals in the database and writing a migration if needed.
• During the Appeal ingestion script, if the incoming data does not have a country attached to the appeal or the country lookup fails, output an error log, but do not attempt to insert that record into the database. For that I guess here instead of returning a None country, we should raise an exception: https://github.com/IFRCGo/go-api/blob/develop/api/management/commands/ingest_appeals.py#L191 and I guess catch the exception here and just skip inserting this record in case of failing country lookup: https://github.com/IFRCGo/go-api/blob/develop/api/management/commands/ingest_appeals.py#L206 ?

We should discuss this - if we think that we want to continue allowing Null values for country on the backend in case the country look-up fails, then we would need to rather make changes on the client to deal with null values for country.

For this particular Appeal where we had the country come in as null, we should investigate why that happened: was this information missing in the upstreeam data, or did we fail to match the country code correctly, or .. ?

cc @frozenhelium @thenav56 @szabozoltan69 @tovari

[tovari]

Thanks @batpad . The reason for this specific case was the missing country info in Apple where IFRC manages appeals and which feeds our incoming api data. It was a human error.
Unfortunately, I don't know what value was provided in the api.

[batpad]

Thanks for checking on the data @tovari ! This sounds like we should just enforce NOT NULL on the country column on the Appeals table.

@thenav56 do you know if you'd be able to look at making that change and dealing with migrations?

cc @szabozoltan69

[thenav56]

We will start working on this. Basically, we will need to do this.
cc: @udaynwa @samshara

DB fix:

Looking at the production, we have something like this.

In [2]: from api.models import Appeal, AppealHistory

In [3]: Appeal.objects.filter(country__isnull=True).count()
Out[3]: 0

In [6]: AppealHistory.objects.filter(country__isnull=True).count()
Out[6]: 4

We will need to update Appeal History, then we can easily do the migration change.

Schema fix:

Currently, we have required=False for nullable field in most of the custom fields in serializers. Seems like we will need to use allow_null=True to update the schema.

[batpad]

🙏 Thank you @thenav56 ! Let me know if I can help on fixing those AppealHistory objects?

[szabozoltan69]

I've updated the correct country id-s on Staging and on Prod for the null-Country-having appealhistory records.
(On Staging there were 6, on Prod there were 4 such records, both were related to 2 appeals: for Chad and Mali.)

[szabozoltan69]

We will need to update Appeal History, then we can easily do the migration change.

So this action item is done :-)

[szabozoltan69]

@batpad @thenav56