Feature/geolocation by Eliory09 · Pull Request #193 · PythonFreeCourse/calendar

Eliory09 · 2021-02-03T01:28:10Z

Issue #185 : Events geolocation support:

Calendar suggest an address and coordinates according to the user's input.
Event schema updated - latitude, longitude.
Google Maps object in the event view page.

codecov-io · 2021-02-03T02:45:38Z

Codecov Report

Merging #193 (9d4e1c3) into develop (f85eb84) will increase coverage by 0.00%.
The diff coverage is 93.10%.

@@           Coverage Diff            @@
##           develop     #193   +/-   ##
========================================
  Coverage    95.40%   95.41%           
========================================
  Files           79       79           
  Lines         3549     3575   +26     
========================================
+ Hits          3386     3411   +25     
- Misses         163      164    +1

Impacted Files	Coverage Δ
app/internal/event.py	`95.71% <89.47%> (-2.37%)`	⬇️
app/database/models.py	`97.24% <100.00%> (+0.02%)`	⬆️
app/routers/event.py	`97.09% <100.00%> (+0.43%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f85eb84...9d4e1c3. Read the comment docs.

yammesicka · 2021-02-03T06:44:20Z

app/routers/event.py

+def get_location_coordinates(
+        address: str,
+        timeout: float = LOCATION_TIMEOUT
+        ) -> Tuple[float, float, str]:


The type hinting is incorrect

yammesicka · 2021-02-03T06:44:58Z

app/templates/event/partials/view_event_details_tab.html

 </div>

+{% if event.latitude is not none and event.longitude is not none %}
+    <div style="width: 100%">


Do not use inline styles

yammesicka · 2021-02-03T06:45:03Z

app/templates/event/partials/view_event_details_tab.html

@@ -1,4 +1,4 @@
-<div class="event_info_row title" style="border-bottom: 4px solid {{event.color}}">
+<div class="event_info_row title" style="border-bottom: 4px solid '{{event.color}}'">


Do not use inline styles

…into feature/geolocation

Eliory09 · 2021-02-05T13:21:21Z

Typo issues resolved.
Event-color support added.

ivarshav · 2021-02-05T13:41:31Z

app/database/models.py

    end = Column(DateTime, nullable=False)
    content = Column(String)
    location = Column(String)
+    latitude = Column(String)


Can they be null?

Actually, they do. Thanks.

ivarshav · 2021-02-05T13:42:21Z

app/routers/event.py

                            '%Y-%m-%d %H:%M')
    user = session.query(User).filter_by(id=1).first()
-    user = user if user else create_user("u", "p", "e@mail.com", session)
+    user = user if user else create_user(


Maybe add a function get_user_or_create_default?

ivarshav · 2021-02-05T13:44:21Z

app/routers/event.py

+            return location.latitude, location.longitude, acc_address
+    except (GeocoderTimedOut, GeocoderUnavailable) as e:
+        logger.exception(str(e))
+    return None


You can return (None, None, location), and then you could remove if location_details is not None: in line 64

Good idea 👍

ivarshav · 2021-02-05T13:44:48Z

app/static/event/eventview.css

 	margin-block-end: 0.2em;
 }

-.title {


No one uses that?

It was used in view_event_details_tab.html and nowhere else.
The event color was set to blue by default in class .title.
I removed the class and switched it with a class for each color, according to the events color in the db.

ivarshav · 2021-02-05T13:45:36Z

tests/asyncio_fixture.py

    create_event(
        db=session,
        title='Cool today event',
+        color='red',


Not sure I got that, are the colors related to your feature?

No. While coding, 30 tests were failed because "color" column added to User schema and was pushed but wasn't supported by all tests written so far. Fixed it and added colors to some tests to make sure everything works.

ivarshav · 2021-02-05T13:45:55Z

tests/test_geolocation.py

+
+
+class TestGeolocation:
+


please remove blank line in line 7

ivarshav · 2021-02-05T13:47:18Z

tests/test_geolocation.py

+        assert response.ok
+        url = event_test_client.app.url_path_for('eventview', event_id=1)
+        response = event_test_client.get(url)
+        location = get_location_coordinates(


maybe _, loaction_name, _ = ..

ivarshav · 2021-02-05T13:47:50Z

tests/test_geolocation.py

+        assert response.ok
+        url = event_test_client.app.url_path_for('eventview', event_id=1)
+        response = event_test_client.get(url)
+        location = get_location_coordinates(


Same here :)

yammesicka

Great progress! :) We're almost there

yammesicka · 2021-02-07T05:22:31Z

app/routers/event.py



+def get_user_or_create_default(user: User, session: Session) -> User:
+    """Check if the user is logged in, else create a fictive user


Add a blank line after the docstring's summary line :)

I decided to remove the method, after it was edited by someone else.

yammesicka · 2021-02-07T05:23:43Z

app/static/event/eventview.css

+}
+
+.google_maps_object {
+	width: 100%


; (You missed it in 4 places :))

…into feature/geolocation

…endar into feature/geolocation

yammesicka

Added few insights :)

yammesicka · 2021-02-14T23:16:45Z

app/internal/event.py

+    """Return location coordinates and accurate
+    address of the specified location."""
+    Location = namedtuple('Location', 'latitude, longitude, location')
+    geolocator = Nominatim(user_agent="calendar", timeout=timeout)


Fortunately, this module does support work with async.
Use async with

yammesicka · 2021-02-14T23:17:18Z

app/internal/event.py

+        if geolocation is not None:
+            location = Location(geolocation.latitude,
+                                geolocation.longitude,
+                                geolocation.raw["display_name"])
+            return location


Are these lines can raise an exception? If not, put them outside the try/except

yammesicka · 2021-02-14T23:17:53Z

app/internal/event.py

@@ -1,15 +1,28 @@
+from collections import namedtuple


You're not using this anymore, please remove.

yammesicka · 2021-02-14T23:49:57Z

app/static/event/eventview.css

+}
+
+.google_maps_object {
+	width: 100%;


Please set the indentation in the CSS to 2 spaces

Sure thing, my bad.

Still not resolved :)

yammesicka · 2021-02-14T23:51:20Z

requirements.txt

 mccabe==0.6.1
 mypy==0.790
 mypy-extensions==0.4.3
+nest-asyncio==1.5.1


You sure you need this?

I think so. My tests encounter event loop errors if I don't use it.

yammesicka · 2021-02-14T23:51:30Z

tests/__init__.py

@@ -0,0 +1,2 @@
+import nest_asyncio


Move to conftest.py

yammesicka · 2021-02-14T23:52:13Z

tests/test_geolocation.py

+    async def test_get_location_coordinates_correct(location):
+        # Test geolocation search using valid locations.
+        location = await get_location_coordinates(location)
+        assert all(list(location))


list is redundant

yammesicka · 2021-02-14T23:52:44Z

app/templates/event/partials/view_event_details_tab.html

-    <!--        <span class="icon">PRIVACY</span>-->
-  </div>
+<div class="event_info_row title_color_{{event.color}}" >
+    <div class="event_info_row_start">


Prefer 2 spaces, not 4

…into feature/geolocation

yammesicka

Great progress! I've added few further insights

yammesicka · 2021-02-22T17:06:44Z

app/internal/event.py

+) -> Union[Location, str]:
+    """Return location coordinates and accurate
+    address of the specified location."""
+    Location = namedtuple("Location", "latitude, longitude, location")


Define outside, use typing.NamedTuple

Again, my bad. Fixed.

yammesicka · 2021-02-22T17:07:18Z

app/routers/event.py

+from fastapi import APIRouter, Depends, HTTPException, Request
+from pydantic import BaseModel
+from sqlalchemy.exc import SQLAlchemyError
+from sqlalchemy.orm import Session
+from sqlalchemy.orm.exc import MultipleResultsFound, NoResultFound
+from sqlalchemy.sql.elements import Null
+from starlette import status
+from starlette.responses import RedirectResponse, Response
+from starlette.templating import _TemplateResponse


These lines should appear on the 2nd paragraph. Please use git precommit hooks to order it.

Actually, precommit hooks didn't fix it automatically. I separated the paragraphs.

yammesicka · 2021-02-22T17:07:56Z

app/routers/event.py

        raise_if_zoom_link_invalid(vc_link)
+    else:
+        location_details = await get_location_coordinates(location)
+        if type(location_details) is not str:


not isinstance(location_details, str)

Thanks. Fixed.

yammesicka · 2021-02-22T17:08:28Z

app/static/event/eventview.css

+}
+
+.google_maps_object {
+	width: 100%;


Still not resolved :)

…into feature/geolocation

yammesicka · 2021-02-24T00:34:21Z

app/routers/event.py

@@ -1,5 +1,5 @@
-from datetime import datetime as dt
 import json
+from datetime import datetime as dt


Eliory09 added 7 commits February 2, 2021 13:04

Version 1.00

469900d

add: geolocation support for events

f7942ac

fix: minor fix to eventview page

42f8540

fix: minor changes

924331a

fix: minor fixes

ec804c8

fix: flake8 typo fixes

1da45a6

fix: update requirements.txt

142d32d

Eliory09 changed the base branch from main to develop February 3, 2021 01:47

fix: minor changes

3459d19

yammesicka requested changes Feb 3, 2021

View reviewed changes

Eliory09 added 2 commits February 5, 2021 12:35

Merge branch 'develop' of https://github.com/PythonFreeCourse/calendar …

a7c1244

…into feature/geolocation

fix: minor typo changes, event color support

8a51777

ivarshav reviewed Feb 5, 2021

View reviewed changes

fix: minor changes, tests fixes

340f316

Eliory09 requested a review from yammesicka February 6, 2021 08:54

yammesicka requested changes Feb 7, 2021

View reviewed changes

Eliory09 added 4 commits February 8, 2021 01:07

Merge branch 'develop' of https://github.com/PythonFreeCourse/calendar …

b7991bd

…into feature/geolocation

Merge branch 'develop' of https://github.com/PythonFreeCourse/calendar …

3ce26aa

…into feature/geolocation

fix: minor changes

fff5c85

fix: requirements file

ae10219

Eliory09 requested a review from yammesicka February 8, 2021 08:39

Eliory09 added 9 commits February 13, 2021 11:03

fix: minor changes

f83e939

fix: minor typo changes

da783b2

fix: minor typo changes

de0bb59

Merge branch 'develop' of https://github.com/PythonFreeCourse/calendar …

850b773

…into feature/geolocation

fix: minor change.

b1874cc

fix: minor changes.

4809563

Merge branch 'feature/geolocation' of https://github.com/Eliory09/cal…

e104964

…endar into feature/geolocation

fix: typo change.

b46e737

fix: updated requirements.txt

79e5bad

Eliory09 requested a review from yammesicka February 14, 2021 14:07

yammesicka requested changes Feb 14, 2021

View reviewed changes

Eliory09 added 9 commits February 20, 2021 13:29

Merge branch 'develop' of https://github.com/PythonFreeCourse/calendar …

69a22d6

…into feature/geolocation

Merge branch 'develop' of https://github.com/PythonFreeCourse/calendar …

d263102

…into feature/geolocation

fix: minor fixes, async support

8d847f9

Merge branch 'develop' of https://github.com/PythonFreeCourse/calendar …

2474c14

…into feature/geolocation

fix: minor tests fixes

c082ee2

Merge branch 'develop' of https://github.com/PythonFreeCourse/calendar …

2b59fb0

…into feature/geolocation

fix: minor code changes, improve async support

489fbbc

fix: aiohttp added to requirements.txt

8f69b28

fix: requirements.txt

3d9ebec

Eliory09 requested a review from yammesicka February 22, 2021 02:03

yammesicka reviewed Feb 22, 2021

View reviewed changes

Eliory09 added 2 commits February 22, 2021 23:23

fix: minor fixes

555d577

fix: bug fixes

392a07e

Eliory09 requested a review from yammesicka February 22, 2021 21:50

Eliory09 added 2 commits February 22, 2021 23:55

fix: typo

8041d90

Merge branch 'develop' of https://github.com/PythonFreeCourse/calendar …

f33dcee

…into feature/geolocation

yammesicka requested changes Feb 24, 2021

View reviewed changes

fix: reordered imports

9d4e1c3

yammesicka merged commit b947751 into PythonFreeCourse:develop Feb 24, 2021

		@@ -1,4 +1,4 @@
		<div class="event_info_row title" style="border-bottom: 4px solid {{event.color}}">
		<div class="event_info_row title" style="border-bottom: 4px solid '{{event.color}}'">



		def get_user_or_create_default(user: User, session: Session) -> User:
		"""Check if the user is logged in, else create a fictive user

Conversation

Eliory09 commented Feb 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Feb 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yammesicka Feb 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yammesicka Feb 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Eliory09 commented Feb 5, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yammesicka left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yammesicka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Eliory09 Feb 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Eliory09 commented Feb 3, 2021 •

edited

Loading

codecov-io commented Feb 3, 2021 •

edited

Loading

yammesicka Feb 3, 2021 •

edited

Loading

yammesicka Feb 3, 2021 •

edited

Loading

yammesicka left a comment •

edited

Loading

Eliory09 Feb 20, 2021 •

edited

Loading