Refactoring to Django3.2#192
Conversation
Beginning migration to Django3.2 following directives from https://github.com/cookiecutter/cookiecutter-django: * Django3.2: Split settings into several files according with environments * OpenLayers 6.12 * Vue 2.6.14
| kw = { | ||
| "table": "osm_points_of_interest", | ||
| "epsg": 4326, | ||
| "limit": 200, | ||
| } | ||
| try: | ||
| kw["xmin"], kw["ymin"], kw["xmax"], kw["ymax"] = request.GET["ext"].split(",") | ||
| except (KeyError, ValueError): | ||
| pass | ||
|
|
||
| osm_query = create_osm_to_geojson_query(**kw) |
There was a problem hiding this comment.
I'm having difficulty understanding this change. What is the goal of this diff?
| onMapPostCompose({ vectorContext, frameState }) { | ||
| }, | ||
| fetchOsmData: function () { | ||
| this.isFetching = true; | ||
| let map = this.$refs.map.$map; | ||
| let url = url_fetch_osm; | ||
| let currentExtent = map.getView().calculateExtent(map.getSize()); | ||
| let projectionCode = map.getView().getProjection().getCode(); | ||
| let transformExtent = ol.proj.transformExtent(currentExtent, projectionCode, 'EPSG:4326'); | ||
| url += "?ext=" + transformExtent.toString() | ||
| //console.log(url); | ||
| fetch(url) | ||
| .then(response => response.json()) | ||
| .then((data) => { | ||
| if (data.features) { | ||
| this.osmData = data; | ||
| } else { | ||
| console.log("There is no features") | ||
| } | ||
| this.isFetching = false; | ||
| }); | ||
| }, | ||
| onMapMoveStart() { | ||
|
|
||
| }, | ||
| onMapMoveEnd() { | ||
| if (this.isFetching) { | ||
| return | ||
| } | ||
| this.fetchOsmData(); | ||
| }, | ||
| onMapMounted() { | ||
| // now ol.Map instance is ready and we can work with it directly | ||
| this.$refs.map.$map.getControls().extend([ | ||
| new ol.control.ScaleLine(), | ||
| new ol.control.FullScreen(), | ||
| new ol.control.OverviewMap({ | ||
| collapsed: true, | ||
| collapsible: true, | ||
| }), | ||
| new ol.control.ZoomSlider(), | ||
| ]); | ||
| this.$refs.map.$map.updateSize(); | ||
| }, | ||
| onMapClicked(e) { | ||
| console.log(e.coordinate); | ||
| }, |
There was a problem hiding this comment.
These changes look good, but may exceed the scope of a "Django 3.2 refactor" PR. Let's leave them for now, since they probably don't block the review process.
| {% block js_extra %} | ||
| <script type="text/javascript"> | ||
| let api_login = "{% url 'rest_login' %}"; | ||
| let login_error_txt="{% trans 'El usuario y/o la contraseña que especificaste no son correctos' %}."; |
There was a problem hiding this comment.
The localisation base language is English. I believe this string will get added to the English strings, which may be confusing for content translators. I believe we should use the English equivalent of this message in the login template.
| $(document).ready(function () { | ||
| $('#f_login').on("submit", function(ev){ | ||
| $("#f_login_error").hide(); | ||
| ev.preventDefault(); | ||
| const data={ "username": $("#id_login").val(), 'password': $("#id_password").val()} | ||
| fetch(api_login, { | ||
| method: 'POST', // or 'PUT' | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| body: JSON.stringify(data), | ||
| }) | ||
| .then(response => { | ||
| if (response.status==400){ | ||
| $("#f_login_error_txt").html(login_error_txt); | ||
| $("#f_login_error").show(); | ||
| } else { | ||
| is_logged = true; | ||
| } | ||
| response.json() | ||
| }) | ||
| .then(data => { | ||
| if ((data) || (is_logged)) { | ||
| let next = $("input[name=next]").val(); | ||
| if (next === undefined) { | ||
| next = "/" | ||
| } | ||
| window.location.href=next; | ||
| } | ||
| }) | ||
| .catch((error) => { | ||
| $("#f_login_error_txt").html(error); | ||
| $("#f_login_error").show(); | ||
| }); | ||
| }) | ||
| }); |
There was a problem hiding this comment.
This seems like more than just a refactor of the login process. Should we split this to a separate PR?
|
|
||
|
|
||
| class AccountAdapter(DefaultAccountAdapter): | ||
| def is_open_for_signup(self, request: HttpRequest): |
There was a problem hiding this comment.
We probably don't need a feature to block signups. Let's focus this pull request on Django 3.2, actually Django 4 now, compatibility :-)
| @@ -0,0 +1,19 @@ | |||
| # Django | |||
| # ------------------------------------------------------------------------------ | |||
| django==3.2.11 # pyup: < 4.0 # https://www.djangoproject.com/ | |||
There was a problem hiding this comment.
Let's consider migrating to Django 4.x now :-)
| mypy==0.931 # https://github.com/python/mypy | ||
| django-stubs==1.9.0 # https://github.com/typeddjango/django-stubs |
There was a problem hiding this comment.
These changes for static analysis look great, but may be out of scope for the current PR :-)
brylie
left a comment
brylie
left a comment
There was a problem hiding this comment.
Overall, this PR looks great! Let's reconsider a couple of the changes that are beyond the scope of refactoring, such as those I've mentioned in previous comments.
2 participants
Beginning migration to Django3.2 following directives from https://github.com/cookiecutter/cookiecutter-django: