fix: Add a decorator to put API user in request #1278

[vanlummelhuizen]

I tried to create a solution that is elegant and reusable.

[vanlummelhuizen]

A remark: as it is now implemented there are no errors that could be passed to the client like what happens in

Global-signbank/signbank/api_interface.py

Line 100 in fb97037

results['errors'] = [gettext("Your Authorization Token does not match anything.")]

I think I can implement it so that errors could be passed. I will work on that next week.

[Woseseltops]

Fancy walrus operator

[vanlummelhuizen]

Fancy walrus operator

Nice that you noticed. Unfortunately (?) I removed it in my latest changes.

I think this is ready to merge now, if approved of course.

[vanlummelhuizen]

I think this is ready to merge now, if approved of course.

Hmm, I keep finding things to improve in the API. Perhaps keep this PR open until I have finished checking all API endpoints.

[susanodd]

Looks good to me! Much cleaner.

[vanlummelhuizen]

@Woseseltops @susanodd @Jetske

I have gone over all endpoint as documented in https://github.com/Signbank/Global-signbank/wiki/Signbank-API and added the put_api_user_in_request decorator and changed some code where applicable.

Note: The following function seems to need some work. For instance, the errors variable is a string that is never send back to the user.

Global-signbank/signbank/api_interface.py

Lines 468 to 505 in fb97037

	def upload_videos_to_glosses(request, datasetid):
	# get file as a url parameter: /dictionary/upload_videos_to_glosses/5
	has_permission = True
	errors = ""
	if not request.user.is_authenticated:
	has_permission = False
	errors = 'Please login to use the requested functionality.'
	# check if the user can manage this dataset
	try:
	group_manager = Group.objects.get(name='Dataset_Manager')
	except ObjectDoesNotExist:
	group_manager = None
	has_permission = False
	errors = 'No group Dataset_Manager found.'
	groups_of_user = request.user.groups.all()
	if has_permission and group_manager not in groups_of_user:
	has_permission = False
	errors = 'You must be in group Dataset Manager to upload a zip video archive.'
	if has_permission and not errors:
	dataset_id = int(datasetid)
	dataset = Dataset.objects.filter(id=dataset_id).first()
	# get paths of uploaded videos to import
	video_file_paths = uploaded_video_filepaths(dataset)
	else:
	# if the user does not have permission, an empty file is generated
	video_file_paths = []
	pseudo_buffer = VideoImporter()
	return StreamingHttpResponse(
	(pseudo_buffer.write(request, vg) for vg in json_start() + video_file_paths + json_finish()),
	content_type="application/json",
	headers={"Content-Disposition": 'attachment; filename='+'glosses.json'},
	)

[vanlummelhuizen]

Ready to merge afaic

[susanodd]

@Woseseltops @susanodd @Jetske

I have gone over all endpoint as documented in https://github.com/Signbank/Global-signbank/wiki/Signbank-API and added the put_api_user_in_request decorator and changed some code where applicable.

Note: The following function seems to need some work. For instance, the errors variable is a string that is never send back to the user.

Global-signbank/signbank/api_interface.py

Lines 468 to 505 in fb97037

	def upload_videos_to_glosses(request, datasetid):
	# get file as a url parameter: /dictionary/upload_videos_to_glosses/5
	has_permission = True
	errors = ""
	if not request.user.is_authenticated:
	has_permission = False
	errors = 'Please login to use the requested functionality.'
	# check if the user can manage this dataset
	try:
	group_manager = Group.objects.get(name='Dataset_Manager')
	except ObjectDoesNotExist:
	group_manager = None
	has_permission = False
	errors = 'No group Dataset_Manager found.'
	groups_of_user = request.user.groups.all()
	if has_permission and group_manager not in groups_of_user:
	has_permission = False
	errors = 'You must be in group Dataset Manager to upload a zip video archive.'
	if has_permission and not errors:
	dataset_id = int(datasetid)
	dataset = Dataset.objects.filter(id=dataset_id).first()
	# get paths of uploaded videos to import
	video_file_paths = uploaded_video_filepaths(dataset)
	else:
	# if the user does not have permission, an empty file is generated
	video_file_paths = []
	pseudo_buffer = VideoImporter()
	return StreamingHttpResponse(
	(pseudo_buffer.write(request, vg) for vg in json_start() + video_file_paths + json_finish()),
	content_type="application/json",
	headers={"Content-Disposition": 'attachment; filename='+'glosses.json'},
	)

Hmmm. I see that. It looks like it might be testing things that are already tested at a previous step.
Sometimes, to make type check etc it is needed to check for things that don't happen.
(Not related, but an example of, when we know something is unique, but use "first" anyway and ignore the case when there could have been multiple objects. Then test if first is empty. There is lots of code that does this for the dataset languages.)

But could it happen I guess if somebody knew about the url and didn't follow the previous steps.
But it looks like I just ignored these, but tested anyway. (Remove code? Or give a Http warning and crash out?)

[susanodd]

I actually don't know how to "approve" on GitHub. I don't find any place I can click. Normally I just write comments.

[Woseseltops]

Improves maintainability

[susanodd]

Ready to merge afaic

Okay, it's probably time.