Improve parse_role's error handling

[iaindillingham]

parse_role accepts a dotted path to a role class and returns the role class itself (jobserver/authorization/roles.py). As you can see below, it validates the dotted path and calls import_string, which raises an ImportError if the import fails.

job-server/jobserver/authorization/parsing.py

Lines 4 to 24 in aa5e326

	def _ensure_role_paths(paths):
	"""
	Paths must be for one of our Role Classes in jobserver.authorization.roles.
	"""
	EXPECTED_PREFIX = "jobserver.authorization.roles"
	invalid_paths = [p for p in paths if not p.startswith(EXPECTED_PREFIX)]
	if not invalid_paths:
	return
	msg = f"Some Role paths did not start with {EXPECTED_PREFIX}:"
	for path in invalid_paths:
	msg += f"\n - {path}"
	raise ValueError(msg)
	def parse_role(path):
	_ensure_role_paths([path])
	return import_string(path)

If a role class is removed, and dotted paths to that role class are present in the database, then parse_role will raise an ImportError when called. This seems most likely when the role_description template tag is called; in these cases, the user would receive a 500 error, and we would receive a notification from Sentry.

We were tripped up by this issue when we deployed #4636. Because the merge, the deployment, and the notification happened in quick succession, it was easy to back out (#4638). However, the notifications could have been clearer. See, for example: https://ebm-datalab.sentry.io/issues/5935887041/. For this reason, I think we should use a custom error with a more informative error message. Doing so would involve something like...

def parse_role(path):
    _ensure_role_paths([path])

    try:
        return import_string(path)
    except ImportError as error:
        raise CustomError("More informative error message") from error

[Jongmassey]

I think this commit at least partially addresses this but could certainly be extended further with better yet error handling

[mikerkelly]

This had to be at least partially addressed to enable #4639, which was the second deploy towards #4604 after #4636. Otherwise the DB with the old role name couldn't run the latest code and the migration.

The exception now gets handled and the unrecognised role will get saved back to the database, so I'm unsure if there's anything remaining to do in this issue.