search

georchestra / mapstore2-cadastrapp

repository for the mapstore2 version of cadastrapp
GNU General Public License v3.0
2 stars 10 forks source link

remove duplicated quote to remove underscore create by browser #152

Closed pierrejego closed 2 years ago

pierrejego commented 2 years ago

Link to https://github.com/georchestra/cadastrapp/issues/615 and #143

Tested on gis.jdev.fr environnement

MaelREBOUX commented 2 years ago

👍

catmorales commented 2 years ago

For the "_" , use case is decribed in https://github.com/georchestra/cadastrapp/issues/615. However, i will update mapstore2-georchestra, mapstore2-cadastrapp and cadastrapp to test with the latest versions of both.

tdipisa commented 2 years ago

@catmorales are you able to reproduce the same using the context in our DEV instance?

catmorales commented 2 years ago

@catmorales are you able to reproduce the same using the context in our DEV instance?

@tdipisa , No on your dev instance i don't reproduce it because i think that you are not on the last cadastrapp back end version. Cadastrapp BE has been updated for using best recent geotools version and other things. That's why we have these problems and why @pierrejego did this pull request.

I reproduce it on portail-test , you can try it in the "master-urba-foncier" context. with the "_" on chrome, i have updated all this morning in portail-test

Cadastrapp (BE) --> Cadastrapp V2.0 Mapstore2-cadastrapp --> rc18 Mapstore2-georchestra --> 21.02.xx (image docker geosolutions)

catmorales commented 2 years ago

To clarify i will give you details and uses cases on:

pierrejego commented 2 years ago

Hi @tdipisa , this problem came only with new backend. We have changed technologie from Jaxrs to SpringMVC. this made a change on headers. He have additionnal quote " in filename field. This is ok with rfc http https://www.rfc-editor.org/rfc/rfc6266 but not managed with Chrome Engine which replace quote (and special chars) by underscore _ This PR is just made to remove those chars so we don't have any underscore.

We have discuss about it yesterday afternoon with Catherine, Those few change won't have any impact on current version. I therefore allow myself to merge the PR. Do not hesitate to contact me if needed

Regards

tdipisa commented 2 years ago

Hi @tdipisa , this problem came only with new backend. We have changed technologie from Jaxrs to SpringMVC. this made a change on headers. He have additionnal quote " in filename field. This is ok with rfc http https://www.rfc-editor.org/rfc/rfc6266 but not managed with Chrome Engine which replace quote (and special chars) by underscore _ This PR is just made to remove those chars so we don't have any underscore.

We have discuss about it yesterday afternoon with Catherine, Those few change won't have any impact on current version. I therefore allow myself to merge the PR. Do not hesitate to contact me if needed

Regards

Dear @pierrejego ok. In my opinion we should better keep track of this at least by putting a comment in the code for future reference including also the link to this PR and maybe also a link to a new issue aimed to investigate the root cause of the problem for a definitive fix.

landryb commented 2 years ago

In my opinion we should better keep track of this at least by putting a comment in the code for future reference including also the link to this PR and maybe also a link to a new issue aimed to investigate the root cause of the problem for a definitive fix.

from my understanding there's no other "definitive fix", the root cause is identified: underlying libraries changed, adding the quotes is the behaviour of the new library upon which we have no control, so things will be this way and have to be dealt with. The commit doesn't break compatibility with previous versions (filenames are generated by the backend and cant contain user-generate strings so no risk of having " or ' in there), so things are safe for users using the older backend (as you witnessed since you didnt manage to reproduce the issue) - if you update the plugin/client side on your install things will still work as before.

as for putting comments, i think everything is properly "documented" in commit messages which link back to this PR which has all the references needed for related issues.