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

Fix assorted issues in the OpenKlant2 service #1525

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

swrichards
Copy link
Collaborator

No description provided.

@swrichards swrichards force-pushed the swr/demo-2024-12-10-fixes branch 3 times, most recently from 098abde to 92c8514 Compare December 10, 2024 16:52
@swrichards swrichards force-pushed the swr/demo-2024-12-10-fixes branch from 92c8514 to d948da3 Compare December 11, 2024 13:55
@swrichards swrichards requested a review from pi-sigma December 11, 2024 13:56
@@ -1133,7 +1133,7 @@ def create_question(
if len(question.rstrip()) == 0:
raise ValueError("You must provide a question")

if self.mijn_vragen_actor is None:
if self.config.mijn_vragen_actor is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

We check for and set the mijn_vragen_actor in the __init__ of the service, although we didn't provide for the case when the actor is not set in the config, hence the old cold would give an AttributeError. Alternatively, you could update the class attributes with mijn_vragen_actor: str | uuid.UUID | None = None, then the old check if self.mijn_vragen_actor is None could remain.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fair point, but I suggest we keep this for now. I wanted to stay close to parity and the extra check can't hurt, but when we revamp the config we should reconsider these constraints.

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 40.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 94.25%. Comparing base (06541ab) to head (d948da3).
Report is 2 commits behind head on develop.

Files with missing lines Patch % Lines
src/open_inwoner/openklant/services.py 25.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1525      +/-   ##
===========================================
- Coverage    94.25%   94.25%   -0.01%     
===========================================
  Files         1068     1068              
  Lines        40465    40466       +1     
===========================================
  Hits         38141    38141              
- Misses        2324     2325       +1     

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

@swrichards swrichards marked this pull request as ready for review December 11, 2024 14:40
@alextreme alextreme merged commit 235ec5a into develop Dec 11, 2024
20 checks passed
@alextreme alextreme deleted the swr/demo-2024-12-10-fixes branch December 11, 2024 19:51
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.

4 participants