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

✨ [#183] added afgehandeld_op field for internetaken #185

Merged
merged 4 commits into from
May 22, 2024

Conversation

bart-maykin
Copy link
Contributor

Fixes #183

  • Added afgehandeld_op field for internetaken
  • Automaticly set afgehandeld_op when status is set on verwerkt
  • Automaticly clear afgehandeld_op when status is set back on te_verwerken if afgehandeld_op already had a date

@bart-maykin bart-maykin force-pushed the feature/183-internetaak-afgehandeld branch from 3257edc to d02cf9b Compare April 25, 2024 12:42
@bart-maykin bart-maykin requested a review from Coperh April 25, 2024 12:43
@bart-maykin bart-maykin marked this pull request as ready for review April 25, 2024 12:43
@stevenbal stevenbal self-requested a review April 26, 2024 08:12

validated_data["actor"] = Actor.objects.get(uuid=actor_uuid)
validated_data["klantcontact"] = Klantcontact.objects.get(
uuid=klantcontact_uuid
)

if validated_data.get("status") == Taakstatus.verwerkt:
validated_data["afgehandeld_op"] = afgehandeld_op
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it necessary to implement this in the serializer .create and .update? I think it would be easier to implement it in the InterneTaak model .save.

I think since afgehandeldOp is a field for which the value is always now() or None, it should probably be a read-only field on the serializer, that way you can remove the .validate and the changes in .create and .update and leave the logic in InterneTaak.save as is

Copy link
Contributor Author

@bart-maykin bart-maykin Apr 30, 2024

Choose a reason for hiding this comment

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

Voor afgehandeldOp we want the user to either provide a value or user the date when the request was made. Hence why we shouldn't set the field as read_only. We can definitely set the value if none provided in the model .save method.

@bart-maykin bart-maykin force-pushed the feature/183-internetaak-afgehandeld branch from 2384355 to 2199b1a Compare April 30, 2024 12:25
Copy link
Contributor

@Coperh Coperh left a comment

Choose a reason for hiding this comment

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

Some minor suggestions to but looks good.

Copy link
Collaborator

@stevenbal stevenbal left a comment

Choose a reason for hiding this comment

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

Looks good, the final comment about the error message is not super critical, but I think it would be good to keep it in line with the conditional

@bart-maykin bart-maykin force-pushed the feature/183-internetaak-afgehandeld branch from d9ff5ca to 2a0483e Compare May 22, 2024 10:11
@bart-maykin bart-maykin merged commit f1413aa into master May 22, 2024
15 checks passed
@bart-maykin bart-maykin deleted the feature/183-internetaak-afgehandeld branch May 22, 2024 10:18
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.

Kunnen zien wanneer een interne taak is afgehandeld
3 participants