Metadata

[rayson1223]

resolve #17 Include metadata to image level

[Akasch]

Please excuse the delay, I had forgotten about this.

Overall looks good I think, but there are some minor issues to be fixed.

The errors from CI are:

imagetagger/imagetagger/images/forms.py:3:1: F401 'imagetagger.images.models.Image' imported but unused
imagetagger/imagetagger/images/forms.py:14:1: W293 blank line contains whitespace
imagetagger/imagetagger/images/views.py:43:1: F811 redefinition of unused 'json' from line 29
imagetagger/imagetagger/images/views.py:213:1: W293 blank line contains whitespace
imagetagger/imagetagger/images/views.py:277:69: F821 undefined name 'image_file'
imagetagger/imagetagger/images/views.py:407:19: E222 multiple spaces after operator
imagetagger/imagetagger/images/views.py:412:1: E302 expected 2 blank lines, found 1

could you fix them also pleas?

I have not reviewed the changes due to the indention fixes on the template in detail.

[rayson1223]

Hi, Sure, will update the commits accordingly. Thank you for the update as well =) From Rayson

…

On Mon, Jan 7, 2019 at 8:26 PM Nils Rokita ***@***.***> wrote: ***@***.**** requested changes on this pull request. Please excuse the delay, I had forgotten about this. Overall looks good I think, but there are some minor issues to be fixed. The errors from CI are: imagetagger/imagetagger/images/forms.py:3:1: F401 'imagetagger.images.models.Image' imported but unused imagetagger/imagetagger/images/forms.py:14:1: W293 blank line contains whitespace imagetagger/imagetagger/images/views.py:43:1: F811 redefinition of unused 'json' from line 29 imagetagger/imagetagger/images/views.py:213:1: W293 blank line contains whitespace imagetagger/imagetagger/images/views.py:277:69: F821 undefined name 'image_file' imagetagger/imagetagger/images/views.py:407:19: E222 multiple spaces after operator imagetagger/imagetagger/images/views.py:412:1: E302 expected 2 blank lines, found 1 could you fix them also pleas? I have not reviewed the changes due to the indention fixes on the template in detail. ------------------------------ In imagetagger/imagetagger/images/views.py <#110 (comment)>: > from imagetagger.users.forms import TeamCreationForm from imagetagger.users.models import User, Team from imagetagger.tagger_messages.forms import TeamMessageCreationForm +import json Json is already imported on line 43 ------------------------------ In imagetagger/imagetagger/annotations/static/annotations/js/annotations.js <#110 (comment)>: > @@ -17,6 +17,7 @@ globals = { mouseDownY: undefined, currentAnnotations: undefined, allAnnotations: undefined, + allMetadata: undefined, please fix the indention errors ------------------------------ In imagetagger/imagetagger/images/views.py <#110 (comment)>: > @@ -43,6 +45,35 @@ from datetime import date, timedelta ***@***.***_required +def metadata_create(request): + if request.method == 'POST': + form = ImageMetadataForm(request.POST) + if form.is_valid(): + data = request.POST please do not use request.POST directly use form.cleaned_data ------------------------------ In imagetagger/imagetagger/images/views.py <#110 (comment)>: > + form = ImageMetadataForm(request.POST) + if form.is_valid(): + data = request.POST + img = get_object_or_404(Image, pk=data['image']) + metadata = json.loads(img.metadata) + metadata[data['name']] = data['value'] + img.metadata = json.dumps(metadata) + img.save() + messages.info(request, + _("Successfully updated \'{}\' in metadata".format(data['name']))) + return redirect(reverse('annotations:annotate', args=(img.pk,))) + return redirect(reverse('annotations:annotate', args=(request.POST['image'],))) + + ***@***.***_required +def metadata_delete(request, image_id): a @require_POST should be used, as this method has to be called via POST ------------------------------ In imagetagger/imagetagger/images/views.py <#110 (comment)>: > + img = get_object_or_404(Image, pk=data['image']) + metadata = json.loads(img.metadata) + metadata[data['name']] = data['value'] + img.metadata = json.dumps(metadata) + img.save() + messages.info(request, + _("Successfully updated \'{}\' in metadata".format(data['name']))) + return redirect(reverse('annotations:annotate', args=(img.pk,))) + return redirect(reverse('annotations:annotate', args=(request.POST['image'],))) + + ***@***.***_required +def metadata_delete(request, image_id): + img = get_object_or_404(Image, id=image_id) + metadata = json.loads(img.metadata) + metadata.pop(request.POST['key']) What happens then the key does not (longer) exists What if POST['key'] is not set and gives an exception? ------------------------------ In imagetagger/imagetagger/images/urls.py <#110 (comment)>: > @@ -9,6 +9,8 @@ url(r'^image/setfree/(\d+)/$', views.set_free, name='setfree_imageset'), url(r'^image/upload/(\d+)/$', views.upload_image, name='upload_image'), url(r'^image/(\d+)/$', views.view_image, name='view_image'), + url(r'^image/metadata/create', views.metadata_create, name='metadata_create'), Please add a /$ ad the end of the url. (All urls should end with a /) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#110 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACATM44EV5UBl3RK1ucP4xq1TOi8SPqcks5vAzz1gaJpZM4ZTbRP> .

[rayson1223]

@Akasch , thank you for the tips and guidance above, it is really helpful!
I had update the code according as requested at the latest commit.
Do let me know if there's anything else required to me update for this pr.

[Akasch]

I think if you change the two minor issues it should be good to merging

[Akasch]

if POST['key'] is not set an exception is thrown

[Akasch]

@login_required is now missing here, just do both on seperate lines:

@login_required
@require_POST
def metadata_delete(request, image_id):