Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

provide an XML2PDF print service proxy #631

Merged
merged 20 commits into from
Jul 19, 2019

Conversation

vvmruder
Copy link
Collaborator

@vvmruder vvmruder commented Sep 5, 2018

This pull request contains changes to XML which are necessary to satisfy XML2PDF print service. This is only a first approach. If we will see too heavy changes we will implement a dedicated XML2PDF renderer to produce such XML as second step.

@vvmruder
Copy link
Collaborator Author

vvmruder commented Sep 5, 2018

with the fix #628 and this changes I was able to generate a PDF
0fc65a05-153e-4de7-806c-7742af465d84.pdf
There is still some problems to solve:

  • GML is not valid in the opinion of XML2PDF service (therefore real estate highlight is not shown)
  • The xlink thing is not necessary in my point of view
  • XML2PDF handles our approach of 'mixable' languages in an extract not properly (that's why some content is lacking).

I'am in contact with GISDATEN AG to find a solution on that.

@pramisters
Copy link
Collaborator

pramisters commented Mar 26, 2019

  • According to GIS Daten AG (20.03.2019) they will solve the xlink issue with the next release of XML2PDF.

  • As XML2PDF expects decoded URLs pyramid_oereb should not encode URLs when a XML dataextract is requested URLs are encoded in XML-Dataextracts #820

@openoereb openoereb deleted a comment May 13, 2019
@openoereb openoereb deleted a comment May 13, 2019
@openoereb openoereb deleted a comment May 13, 2019
@openoereb openoereb deleted a comment Jun 11, 2019
Copy link
Member

@jwkaltz jwkaltz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vvmruder we have tested your code with @pramisters in the Canton Zug installation, and it was working fine - without needing to change the XML templates. We only needed to add your proxy code. Perhaps the xml2pdf service has been adapted in the meantime.

There is an additional issue about missing geometry data, but I am working on a separate PR for that. In my opinion, if we rework your PR such that it only adds the code (without changing the XML templates), we can merge it into master.

@vvmruder
Copy link
Collaborator Author

@jwkaltz This sounds good. I planned to work on that today. So I will rework it and push again. Than it can be merged.

@vvmruder
Copy link
Collaborator Author

@jwkaltz It was coming into my mind that we had an issue with encoded URL's in XML. Is this solved too?

@vvmruder
Copy link
Collaborator Author

@jwkaltz About the codacy-bot issues: I don't know how to solve this, because the import statements are there for inter python version compatibility. Its correct that one of the import statements will not be used... Can you contribute on that?

@jwkaltz
Copy link
Member

jwkaltz commented Jul 18, 2019

@jwkaltz It was coming into my mind that we had an issue with encoded URL's in XML. Is this solved too?

It seems so, yes. Since we used your code without changes, but did not change the XML templates, I am assuming XML2PDF changed something and the encoding is no longer a problem.

@jwkaltz
Copy link
Member

jwkaltz commented Jul 18, 2019

@jwkaltz About the codacy-bot issues: I don't know how to solve this, because the import statements are there for inter python version compatibility. Its correct that one of the import statements will not be used... Can you contribute on that?

It looks to me like urlparse is not used in this file, so these 4 lines can simply be deleted? Or am I missing something?

@vvmruder
Copy link
Collaborator Author

@jwkaltz About the codacy-bot issues: I don't know how to solve this, because the import statements are there for inter python version compatibility. Its correct that one of the import statements will not be used... Can you contribute on that?

It looks to me like urlparse is not used in this file, so these 4 lines can simply be deleted? Or am I missing something?

Ok so will look at it again and remove. Maybe I was mixing something up.

@vvmruder
Copy link
Collaborator Author

@jwkaltz It was coming into my mind that we had an issue with encoded URL's in XML. Is this solved too?

It seems so, yes. Since we used your code without changes, but did not change the XML templates, I am assuming XML2PDF changed something and the encoding is no longer a problem.

So I will remove changes on XML. And commit again.

@vvmruder
Copy link
Collaborator Author

Since this is a actual bug I think we should resolve it soon. The URL's are encoded twice. This leads to problems with the XML handling. But I added a Bug #884

@vvmruder
Copy link
Collaborator Author

Just for clarification #884 will be solved in another PR. Not in this one.

@openoereb openoereb deleted a comment Jul 18, 2019
@openoereb openoereb deleted a comment Jul 18, 2019
@openoereb openoereb deleted a comment Jul 18, 2019
@openoereb openoereb deleted a comment Jul 18, 2019
@openoereb openoereb deleted a comment Jul 18, 2019
@openoereb openoereb deleted a comment Jul 18, 2019
@jwkaltz jwkaltz changed the title WIP: refine XML to be valid against XML2PDF print service provide an XML2PDF print service proxy Jul 18, 2019
@jwkaltz jwkaltz self-requested a review July 18, 2019 15:11
@openoereb openoereb deleted a comment Jul 18, 2019
jwkaltz added 2 commits July 19, 2019 06:24
…oereb/pyramid_oereb into necessary_changes_for_xml2pdf_print
@openoereb openoereb deleted a comment Jul 19, 2019
@openoereb openoereb deleted a comment Jul 19, 2019
@openoereb openoereb deleted a comment Jul 19, 2019
@jwkaltz jwkaltz merged commit b440379 into master Jul 19, 2019
@jwkaltz jwkaltz deleted the necessary_changes_for_xml2pdf_print branch July 19, 2019 05:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants