Incorporate non-standard proposal in pdf workflow #574
Conversation
woops
I'll look at this PR tomorrow in-depth. But for now: Yeah, it's because local dev doesn't have https. Normally, you'd solve this by retrieving the used protocol (http vs https) from the request object; but PDF gen doesn't (always) have access to that. It will be fine on the server, those are HTTPS only. You could maybe create an if on the DEBUG setting, but I don't have any strong opinions tbh |
proposals/models.py
Outdated
| @property | ||
| def pdf_template_name(self): | ||
| """Determine the correct PDf template for this proposal.""" | ||
| template_name = 'proposals/proposal_pdf.html' | ||
| if self.is_pre_approved: | ||
| template_name = 'proposals/proposal_pdf_pre_approved.html' | ||
| elif self.is_pre_assessment: | ||
| template_name = 'proposals/proposal_pdf_pre_assessment.html' | ||
| return template_name | ||
| return 'proposals/proposal_pdf.html' |
There was a problem hiding this comment.
I agree that this is redundant now; differences should be handled by the new PDF framework
There was a problem hiding this comment.
I got rid of this now, and its reference in the ProposalAsPDF view. This works fine for me. I was wondering does this now require a migration?
There was a problem hiding this comment.
No it doesn't. Migrations are only needed for ORM Fields (which are always class variables, and (almost) always named xField). This is a class method, those should never trigger migrations.
If you are unsure, you can run python manage.py makemigrations --dry-run. This will only display the migrations Django wants to create, without actually creating them.
Alternatively, we now have a check for PR's that will tell you if you are missing a migration. The latest run passed successfully: https://github.com/DH-IT-Portal-Development/ethics/actions/runs/6847879373/job/18616976001?pr=574
Fix for issue #572. I got rid of now obsolete pdf templates and made sure the pre-assessment and pre-approval proposals are properly incorporated in the new PDF workflow.
Maybe the
pdf_template_namefunction can be removed entirely? I guess it is not being used in this version. What do y'all think?@tymees I did notice that the change to
httpsin thehandle_field_filefunction of theRowValueClasscauses an error in my browser when clicking links in the pdf, with the error code: SSL_ERROR_RX_RECORD_TOO_LONGHowever, Django tells me this is because the development server only support http:
You're accessing the development server over HTTPS, but it only supports HTTP.So it will be fine on the production server?