efdevcon / pretix-attestation-placeholder-plugin

Pretix plugin to support placeholders for attestations
MIT License
2 stars 7 forks source link

Add KeyFile model #25

Closed Nurts closed 3 years ago

Nurts commented 3 years ago

What was wrong ?

How it was fixed ?

kvbik commented 3 years ago

I believe this direction is correct. The database column can totally work. Now just try to handle the uploaded data the right way using the Form instance (some changes to the Form may be necessary ~ haven't looked too deeply on that).

Nurts commented 3 years ago

Hi @kvbik

Now just try to handle the uploaded data the right way using the Form instance

I am not sure what you meant by handling it the right way. I thought FormView[1] already handles it the right way. Then the storage object, that is specified in KeyFile model [2] writes it to specified place (currently to settings.MEDIA_ROOT + 'pretix_attestation_plugin/keyfiles/').

[1] https://github.com/efdevcon/pretix-attestation-placeholder-plugin/blob/main/pretix_attestation_plugin/views.py#L13 [2] https://github.com/efdevcon/pretix-attestation-placeholder-plugin/blob/bf7eb587402f594b160a024a6c55c97c482b3cf3/pretix_attestation_plugin/models.py#L37-L42

kvbik commented 3 years ago

You partially do, no worries! The place that is (imho) not correct is self.write_to_file(self.request.FILES.get('keyfile', None)), because you are taking raw data from the request object. You want to work with validated data and that should come from the Form instance - which is the cleaned_data variable.

kvbik commented 3 years ago

Thx @Nurts - the last commit is exactly what I meant! Let's merge this and test in production :D :D