Dev#22
Conversation
…nization-model feat: add template field for organization model
…etter fix: prevent crashing using itemgetter
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a template field to the Organization model and updates the initialization logic to handle organization templates. Key feedback includes a critical bug where the --org-template argument is missing from the management command's parser, which will cause the service to fail on startup. Other issues identified include the failure to persist the template value to the database during organization creation and a potential KeyError when accessing user emails in the authentication service.
| return { | ||
| "org_name": kwargs.get("org_name") or os.getenv("ORG_NAME"), | ||
| "org_slug": kwargs.get("org_slug") or os.getenv("ORG_SLUG"), | ||
| "org_template": kwargs.get("org_template") or os.getenv("ORG_TEMPLATE"), |
There was a problem hiding this comment.
The --org-template argument is missing from the add_arguments method in this command. Since docker-entrypoint.sh passes this flag, the command will fail with an 'unrecognized arguments' error, preventing the service from starting.
| # Publish organization event | ||
| self._publish_org_event(org_id, org_slug, org_name, result) | ||
| self._send_celery_task(org_id, org_name, org_slug, user) | ||
| self._send_celery_task(org_id, org_name, org_slug, org_template, user) |
There was a problem hiding this comment.
The org_template value is passed to the Celery task but is not used to populate the template field of the Organization model during creation. The _create_organization_with_roles method should be updated to accept this parameter and include it in the Organization.objects.create call.
| template = models.CharField( | ||
| max_length=256, choices=OrganizationTemplate.choices, blank=True, null=True | ||
| ) |
There was a problem hiding this comment.
The template field is added here, but it is not being populated in the init_organization management command. This field will remain null in the database for newly initialized organizations.
| given_name = user_info_dict.get("given_name", "") | ||
| family_name = user_info_dict.get("family_name", "") | ||
| email = user_info_dict["email"] |
There was a problem hiding this comment.
Accessing email directly from user_info_dict may raise a KeyError if the key is missing from the provider's response. It is safer to use .get() and handle the missing value explicitly, for example by raising a ValidationError.
given_name = user_info_dict.get("given_name", "")
family_name = user_info_dict.get("family_name", "")
email = user_info_dict.get("email")
if not email:
raise ValidationError({"email": f"Email not provided by {provider}."})1 participant
What?
Why?
How?
Testing?
Anything Else?