✨ [#2179] Display list of Qmatic appointments in profiel #1099

stevenbal · 2024-03-19T08:41:11Z

task: https://taiga.maykinmedia.nl/project/open-inwoner/task/2179

Can be tested by logging in with an account with email [email protected] and navigating to Mijn profiel

I adapted the client code from Open Forms and replaced TypedDict with pydantic's BaseModel to deserialize the data. Although we don't actually use the same client methods as OF (because OF implements creating appointments and we just need to implement the list of appointments + modify/cancel), this should probably be merged into a library at some point?

codecov-commenter · 2024-03-19T11:38:05Z

Codecov Report

Attention: Patch coverage is 73.52941% with 36 lines in your changes are missing coverage. Please review.

Project coverage is 94.92%. Comparing base (689245f) to head (e83be31).
Report is 6 commits behind head on develop.

Files	Patch %	Lines
src/open_inwoner/qmatic/client.py	69.31%	27 Missing ⚠️
src/open_inwoner/accounts/views/profile.py	38.46%	8 Missing ⚠️
src/open_inwoner/qmatic/models.py	91.66%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1099      +/-   ##
===========================================
- Coverage    95.02%   94.92%   -0.10%     
===========================================
  Files          911      919       +8     
  Lines        31991    32169     +178     
===========================================
+ Hits         30399    30537     +138     
- Misses        1592     1632      +40

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

task: https://taiga.maykinmedia.nl/project/open-inwoner/task/2179

stevenbal · 2024-03-19T14:30:01Z

src/open_inwoner/templates/pages/profile/appointments.html

+                            {% list_item text=appointment.branch.name caption=_("Locatie") compact=True strong=False %}
+                            {% list_item text=appointment.branch.addressCity compact=True strong=False %}
+                            {% list_item text=appointment.branch.addressLine2 compact=True strong=False %}


I'm not sure if these are the right fields to display, because the documentation isn't too clear about what is stored in these fields

Feels like a testing and acceptance problem 😉

stevenbal · 2024-03-19T14:32:27Z

src/open_inwoner/accounts/views/profile.py

+            context["appointments"] = []
+        else:
+            context["appointments"] = client.list_appointments_for_customer(
+                quote(self.request.user.email)


This assumes that the municipality uses a citizen's email as external_id in Qmatic (still awaiting confirmation from Hoorn if this is correct). If we want to support bsn or something else, we'll probably have to add a feature flag or choicefield on the config to enable that.

This quote() doesn't belong here but should be in the client method. It is not the responsibility of the using code to encode the values (as conceptually it doesn't even know how the client works internally).

This relates to my earlier comment I put in the client.

Bartvaderkin

Pretty clean, with some subtle feedback.

src/open_inwoner/qmatic/client.py

Bartvaderkin · 2024-03-21T10:20:03Z

src/open_inwoner/templates/pages/profile/appointments.html

+                            {% list_item text=appointment.branch.name caption=_("Locatie") compact=True strong=False %}
+                            {% list_item text=appointment.branch.addressCity compact=True strong=False %}
+                            {% list_item text=appointment.branch.addressLine2 compact=True strong=False %}


Feels like a testing and acceptance problem 😉

src/open_inwoner/qmatic/client.py

Bartvaderkin · 2024-03-21T10:40:12Z

src/open_inwoner/accounts/views/profile.py

+            context["appointments"] = []
+        else:
+            context["appointments"] = client.list_appointments_for_customer(
+                quote(self.request.user.email)


This quote() doesn't belong here but should be in the client method. It is not the responsibility of the using code to encode the values (as conceptually it doesn't even know how the client works internally).

This relates to my earlier comment I put in the client.

src/open_inwoner/qmatic/client.py

pi-sigma

Couple minor questions + suggestions

pi-sigma · 2024-03-21T11:03:54Z

src/open_inwoner/accounts/tests/test_profile_views.py

+
+        self.assertEqual(len(cards), 2)
+
+        passport_appointment = PQ(cards[0]).find("ul").children()


Is it necessary to use PyQuery directly, since you're already using response.pyquery, or just more convenient?

It actually is necessary, it's a bit weird but for some reason I can't use .find and other methods on the element that I get from cards[0] directly

This looks familiar: iirc pyquery doesn't wrap recursively like some other api's do.

src/open_inwoner/qmatic/client.py

src/open_inwoner/qmatic/models.py

stevenbal · 2024-03-25T08:48:17Z

@Bartvaderkin can you re-review this? :)

stevenbal marked this pull request as draft March 19, 2024 08:42

stevenbal force-pushed the feature/2178-qmatic-integration branch from 344d0c6 to c0b909e Compare March 19, 2024 10:16

stevenbal changed the title ~~🚧 [#2178] qmatic~~ 🚧 [#2178] Qmatic appointment integration Mar 19, 2024

stevenbal force-pushed the feature/2178-qmatic-integration branch from c0b909e to e83be31 Compare March 19, 2024 11:07

stevenbal force-pushed the feature/2178-qmatic-integration branch from e83be31 to 41f85fc Compare March 19, 2024 11:44

stevenbal changed the title ~~🚧 [#2178] Qmatic appointment integration~~ ✨ [#2178] Qmatic appointment integration Mar 19, 2024

stevenbal force-pushed the feature/2178-qmatic-integration branch from 41f85fc to 6dd5f69 Compare March 19, 2024 11:50

stevenbal changed the title ~~✨ [#2178] Qmatic appointment integration~~ ✨ [#2179] Qmatic appointment integration Mar 19, 2024

stevenbal force-pushed the feature/2178-qmatic-integration branch from 6dd5f69 to a1866a6 Compare March 19, 2024 14:21

stevenbal added 4 commits March 19, 2024 15:24

✨ [#2179] Add app for Qmatic appointment integration

ef744ec

task: https://taiga.maykinmedia.nl/project/open-inwoner/task/2179

🤡 [#2179] Add mock endpoint for Qmatic appointments

bccc944

✨ [#2179] Add Mijn afspraken to Mijn profiel

fcd6fde

✅ [#2179] Add tests for Mijn afspraken in profile

a4e7f8c

stevenbal force-pushed the feature/2178-qmatic-integration branch from a1866a6 to a4e7f8c Compare March 19, 2024 14:24

stevenbal commented Mar 19, 2024

View reviewed changes

stevenbal mentioned this pull request Mar 19, 2024

✨ [#2180] Implement reschedule and delete for Qmatic appointments #1103

Merged

stevenbal changed the title ~~✨ [#2179] Qmatic appointment integration~~ ✨ [#2179] Display list of Qmatic appointments in profiel Mar 19, 2024

stevenbal marked this pull request as ready for review March 21, 2024 08:53

stevenbal requested review from Bartvaderkin and pi-sigma March 21, 2024 09:01

Bartvaderkin suggested changes Mar 21, 2024

View reviewed changes

stevenbal requested a review from Bartvaderkin March 21, 2024 11:35

pi-sigma approved these changes Mar 21, 2024

View reviewed changes

👌 [#2179] PR feedback

6d1313e

stevenbal force-pushed the feature/2178-qmatic-integration branch from 5efd9a9 to 6d1313e Compare March 21, 2024 12:52

Bartvaderkin approved these changes Mar 26, 2024

View reviewed changes

stevenbal merged commit 4928b5d into develop Mar 26, 2024
15 checks passed

stevenbal deleted the feature/2178-qmatic-integration branch March 26, 2024 13:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ [#2179] Display list of Qmatic appointments in profiel #1099

✨ [#2179] Display list of Qmatic appointments in profiel #1099

stevenbal commented Mar 19, 2024 •

edited

Loading

codecov-commenter commented Mar 19, 2024

stevenbal Mar 19, 2024

Bartvaderkin Mar 21, 2024

stevenbal Mar 19, 2024

Bartvaderkin Mar 21, 2024

Bartvaderkin left a comment

Bartvaderkin Mar 21, 2024

Bartvaderkin Mar 21, 2024

pi-sigma left a comment

pi-sigma Mar 21, 2024

stevenbal Mar 21, 2024

Bartvaderkin Mar 21, 2024

stevenbal commented Mar 25, 2024


		self.assertEqual(len(cards), 2)

		passport_appointment = PQ(cards[0]).find("ul").children()

✨ [#2179] Display list of Qmatic appointments in profiel #1099

✨ [#2179] Display list of Qmatic appointments in profiel #1099

Conversation

stevenbal commented Mar 19, 2024 • edited Loading

codecov-commenter commented Mar 19, 2024

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bartvaderkin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pi-sigma left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevenbal commented Mar 25, 2024

stevenbal commented Mar 19, 2024 •

edited

Loading