Review mechanism for newly created locations

[simonw]

Split from #609:

• Create an "Is pending review" checkbox on the Location (like the one on Reports)
• Add a filter for the list of locations to see just locations that are pending review (or not pending review)
• Automatically flag any newly created location for review if the person who added it has the "WB Trainee" auth0 role
• Automatically flag reports for review if the reporter has the "WB Trainee" auth0 role AND the "Report source" is "Data corrections" or "Web banking"

[simonw]

In addition to the above changes, we also need to make sure that locations that are "pending review" are not included in our three different exports: to vaccinateca.com, to vaccinatethestates.com (via Mapbox) and to our new API.

The vaccinatethestates.com, Mapbox and new API cases are covered by this filtering function:

vial/vaccinate/api/search.py

Lines 330 to 344 in 5aaf47f

	def filter_for_export(qs):
	# Filter down to locations that we think should be exported
	# to the public map on www.vaccinatethestates.com
	return qs.exclude(
	dn_latest_non_skip_report__planned_closure__lt=datetime.date.today()
	).exclude(
	dn_latest_non_skip_report__availability_tags__slug__in=(
	"incorrect_contact_information",
	"location_permanently_closed",
	"may_be_a_vaccination_site_in_the_future",
	"not_open_to_the_public",
	"will_never_be_a_vaccination_site",
	"only_staff",
	)
	)

The vaccinateca.com case is covered by this code:

vial/vaccinate/core/exporter/__init__.py

Lines 126 to 136 in 5aaf47f

	ds.locations = (
	models.Location.objects.filter(state__abbreviation="CA", soft_deleted=False)
	.exclude(
	dn_latest_non_skip_report__planned_closure__lt=datetime.date.today()
	)
	.select_related("dn_latest_non_skip_report__appointment_tag")
	.select_related("county")
	.select_related("location_type")
	.select_related("provider")
	.prefetch_related("dn_latest_non_skip_report__availability_tags")
	).only(

We'll need to add unit tests to check this too - existing tests for those are in https://github.com/CAVaccineInventory/vial/blob/5aaf47fa4a12660660df65f545c88611adf18054/vaccinate/api/test_export_mapbox.py and https://github.com/CAVaccineInventory/vial/blob/5aaf47fa4a12660660df65f545c88611adf18054/vaccinate/core/test_api_exporter.py

[simonw]

Manually created locations happen exclusively through the VIAL admin interface - we already have a custom .save_model() method which we can use to check the current user's groups and apply the is_pending_review flag if they are a WB Trainee:

vial/vaccinate/core/admin.py

Lines 580 to 583 in 5aaf47f

	def save_model(self, request, obj, form, change):
	if not change:
	obj.created_by = request.user
	super().save_model(request, obj, form, change)

[rhkeeler]

oh, also: when we create the behavior for the WB Trainee role, we should also change the behavior for the Call Trainee role to only flag reports that have a "Report source" of "Caller app".

[ugotsoul]

Whats the best way to do the migration for backfilling is_pending_review since it introduces a not nullable default value field? I read the suggestion here to use a library to handle adding default values in Django: https://github.com/3YOURMIND/django-add-default-value -- wdyt?

[ugotsoul]

Do I need to worry about airtable users? or have those been migrated to auth0? 🤔

[ugotsoul]

Automatically flag reports for review if the reporter has the "WB Trainee" auth0 role AND the "Report source" is "Data corrections" or "Web banking" Do you mean newly created reports?

[rhkeeler]

Don't need to worry about airtable users! And yes, I mean newly-created reports.

[simonw]

Whats the best way to do the migration for backfilling is_pending_review since it introduces a not nullable default value field? I read the suggestion here to use a library to handle adding default values in Django: https://github.com/3YOURMIND/django-add-default-value -- wdyt?

Set it to False for all existing reports, and make it default False too.

[rhkeeler]

And: confirmed with WB captains today that we actually do want to send locations that are pending QA to the website and other data feeds! (We want to review them for accuracy and phrasing and formatting and so on, but we don't expect them to be wrong, just maybe not in line with our standard process. So we don't want to delay showing them!)

[ugotsoul]

And: confirmed with WB captains today that we actually do want to send locations that are pending QA to the website and other data feeds! (We want to review them for accuracy and phrasing and formatting and so on, but we don't expect them to be wrong, just maybe not in line with our standard process. So we don't want to delay showing them!)

@rhkeeler Ok cool -- then do we need send the is_pending_review field along with exporting the location, so it will be flagged for people in the data feeds?

[ugotsoul]

should I add the new auth0 role in this ticket too? I see Simon did it for #612

based on a convo with @rhkeeler, this isn't needed -- no, I don't think so! The WB Limited role does control access to VIAL, but the Trainee role just says "flag this if the reporter has this role

[ugotsoul]

It seems like reports and locations are not get flagged with is_pending_review correctly: https://discord.com/channels/799147121357881364/813861006718926848/850457698268872734

TLDR: Less than 2% of reports are flagged correctly, and no locations.

[ugotsoul]

For reports and locations the code is not working because it is looking for a WB Trainee group to exist, which is not reflected in vial.

vial/vaccinate/core/admin.py

Lines 598 to 603 in 0ac87bd

	def save_model(self, request, obj, form, change):
	if not change:
	obj.created_by = request.user
	obj.is_pending_review = request.user.groups.filter(
	name="WB Trainee"
	).exists()

[ugotsoul]

should we reflect this auth0 role in vial? the benefit is that this information would live on the user model and be easy to filter in vial if needed.

or should I change the code to lookup the auth0 role on the reporter model (reporter.auth0_role_names)? There's also a bug which I need to fix -- auth0 roles may not be populating on reporters correctly #633

[simonw]

I think we should reflect all of the auth0 roles as VIAL groups - with hindsight that would have been a better initial design for how this all works.

[simonw]

I'd love to be confident that the reporter roles are correctly updated when a user gets new roles in auth0 and then makes a request to a VIAL API with a JWT!

[rhkeeler]

@ugotsoul can you close this when your fix for roles not updating gets deployed?