Cria RawOrganization e refatora Organization #1268

robertatakenaka · 2026-02-02T01:00:59Z

O que esse PR faz?

Fale sobre o propósito do pull request como por exemplo: quais problemas ele soluciona ou quais features ele adiciona.

Onde a revisão poderia começar?

Indique o caminho do arquivo e o arquivo onde o revisor deve iniciar a leitura do código.

Como este poderia ser testado manualmente?

Estabeleça os passos necessários para que a funcionalidade seja testada manualmente pelo revisor.

Algum cenário de contexto que queira dar?

Indique um contexto onde as modificações se fazem necessárias ou passe informações que contextualizam
o revisor a fim de facilitar o entendimento da funcionalidade.

Screenshots

Quando aplicável e se fizer possível adicione screenshots que remetem a situação gráfica do problema que o pull request resolve.

Quais são tickets relevantes?

Indique uma issue ao qual o pull request faz relacionamento.

Referências

Indique as referências utilizadas para a elaboração do pull request.

…ATION_ROLES

…ical levels

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 28 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-02T11:02:20Z

organization/models.py

+    class Meta:
+        verbose_name = _("Organization")
+        verbose_name_plural = _("Organizations")
+        unique_together = [("name", "location", "external_id", "source")]  # NOVO


The unique_together constraint on Organization has been changed from ("name", "acronym", "location") to ("name", "location", "external_id", "source"). However, external_id and source can be NULL/blank, and including nullable fields in a unique constraint can lead to unexpected behavior in some databases.

In PostgreSQL, multiple rows with NULL values in a unique constraint are allowed (NULL is not equal to NULL). This means you could have multiple organizations with the same name and location if external_id or source are NULL, which might not be the intended behavior.

Consider either:

Making external_id and source non-nullable with appropriate defaults

Using a partial unique index that only applies when these fields are not NULL

Reconsidering the unique constraint to not include nullable fields

Suggested change

unique_together = [("name", "location", "external_id", "source")] # NOVO

constraints = [

models.UniqueConstraint(

fields=["name", "location", "external_id", "source"],

condition=Q(external_id__isnull=False, source__isnull=False),

name="uniq_organization_name_location_external_source_not_null",

),

]

Copilot · 2026-02-02T11:02:20Z

organization/models.py

        try:
-            return cls.get(name=name)
+            obj = cls.get(
+                original=original,
+                country=country,
+                state=state,
+                city=city,
+                level_1=level_1,
+                level_2=level_2,
+                level_3=level_3,
+            )
+            obj.updated_by = user
+
+            # Atualiza campos se fornecidos
+            if name:
+                obj.name = clean_xml_tag_content(name)
+            if normalized_name:
+                obj.normalized_name = clean_xml_tag_content(normalized_name)
+            if acronym:
+                obj.acronym = clean_xml_tag_content(acronym)
+            if level_1:
+                obj.level_1 = clean_xml_tag_content(level_1)
+            if level_2:
+                obj.level_2 = clean_xml_tag_content(level_2)
+            if level_3:
+                obj.level_3 = clean_xml_tag_content(level_3)
+            if match_status:
+                obj.match_status = match_status
+            if organizational_level:
+                obj.organizational_level = organizational_level
+            if organization:
+                obj.organization = organization
+
+            obj.save()
+            return obj
+
        except cls.DoesNotExist:
-            return cls.create(name=name, user=user)
+            return cls.create(
+                user=user,
+                original=original,
+                country=country,
+                state=state,
+                city=city,
+                level_1=level_1,
+                level_2=level_2,
+                level_3=level_3,
+                name=name,
+                normalized_name=normalized_name,
+                acronym=acronym,
+                match_status=match_status,
+                organizational_level=organizational_level,
+                organization=organization,
+            )


The RawOrganization.create_or_update method has inconsistent parameter handling. In the update path (lines 971-988), it checks if name:, if normalized_name:, etc. before updating, but in the create path (lines 994-1009), these same parameters are passed directly without null checks.

This means:

If name=None is passed to create_or_update on an existing record, the name won't be updated (due to the if name: check)

But if creating a new record with name=None, it will raise a ValueError at line 889-890

This inconsistency can lead to confusion. The update logic should either always update fields when provided (including None values) or should have the same validation as create.

Copilot · 2026-02-02T11:02:20Z

organization/models.py

+    Classe base para níveis organizacionais hierárquicos (versão 2)

-        return obj
+    DEPRECATED: usar BaseOrganizationalLevel em vez disso, pois classe Base não deveria usar CommonControlField


The deprecation notice states to use BaseOrganizationalLevel instead of BaseOrgLevel, but this is misleading. Looking at the code:

BaseOrgLevel (line 192) includes CommonControlField which adds creator/updated_by/timestamps

BaseOrganizationalLevel (line 416) is a plain abstract Model without CommonControlField

If someone follows this deprecation notice and switches from BaseOrgLevel to BaseOrganizationalLevel, they would lose the audit trail fields (creator, updated_by, created, updated). This could break existing code that relies on these fields.

The deprecation notice should clarify:

Why BaseOrgLevel is deprecated despite having more functionality

What the migration path is for code that needs the audit fields

Or reconsider whether BaseOrganizationalLevel should also include CommonControlField

Copilot · 2026-02-02T11:02:21Z

organization/models.py

 from .exceptions import (
-    OrganizationCreateOrUpdateError,
-    OrganizationGetError,
    OrganizationLevelGetError,
    OrganizationTypeGetError,
 )


The exception imports OrganizationCreateOrUpdateError and OrganizationGetError are being removed (lines 19-21), but these exceptions are still defined in the exceptions module. The removed imports suggest these are no longer needed, but it would be good to verify:

Are these exceptions still used elsewhere in the codebase?

Should they also be removed from the exceptions module?

Or should the imports be kept if they're needed by other modules that import from organization.models?

A search of the codebase would help determine if removing these imports is safe.

Copilot · 2026-02-02T11:02:21Z

organization/models.py

+    class Meta:
+        verbose_name = _("Organization")
+        verbose_name_plural = _("Organizations")
+        unique_together = [("name", "location", "external_id", "source")]  # NOVO


The model comment at line 161 includes "# NOVO" (# NEW) which appears to be a development note. These types of annotations should be removed before committing as they don't provide value to future code readers and make the codebase look unfinished.

Comments in production code should explain "why" something is done, not track the development process.

Suggested change

unique_together = [("name", "location", "external_id", "source")] # NOVO

unique_together = [("name", "location", "external_id", "source")]

Copilot · 2026-02-02T11:02:26Z

organization/models.py

+            | Q(source=search_term)
+            | Q(external_id=search_term)


The autocomplete_custom_queryset_filter method for Organization (lines 169-175) searches on source and external_id using exact match: Q(source=search_term) | Q(external_id=search_term).

This means the autocomplete will only match if the user types the exact source value (e.g., "user", "legacy", "MEC", "ror") or the exact external_id. For a better user experience with autocomplete, these should use icontains:

Q(source__icontains=search_term) | Q(external_id__icontains=search_term)

This would make the search more flexible and user-friendly. The exact match on these fields is too restrictive for autocomplete functionality.

Suggested change

| Q(source=search_term)

| Q(external_id=search_term)

| Q(source__icontains=search_term)

| Q(external_id__icontains=search_term)

Copilot · 2026-02-02T11:02:27Z

organization/models.py

+    Usado em:
+        - journal.models: ?
+        - article.models: ?
+


The docstring for RawOrganization (lines 622-656) contains question marks in the "Usado em:" (Used in:) section:

Usado em: - journal.models: ? - article.models: ?

This indicates incomplete documentation. Before merging, the actual usage should be documented, or these lines should be removed if the model is not yet used in those modules. Incomplete documentation with placeholders should not be committed to production code.

Suggested change

Usado em:

- journal.models: ?

- article.models: ?

Copilot · 2026-02-02T11:02:27Z

organization/models.py

    def create_or_update(
        cls,
-        name,
        user,
+        original,
+        country,
+        state,
+        city,
+        level_1,
+        level_2,
+        level_3,
+        name=None,
+        normalized_name=None,
+        acronym=None,
+        match_status=None,
+        organizational_level=None,
+        organization=None,
    ):


In the RawOrganization.create_or_update method, the required parameters are user, original, country, state, city, level_1, level_2, level_3, but name is marked as optional with default name=None.

However, the create method (line 889-890) requires name and raises ValueError if it's not provided. This creates an inconsistency where create_or_update can be called successfully in the update path with name=None, but will fail in the create path.

The method signature should either:

Make name a required parameter in create_or_update

Or remove the validation in create to allow name=None (though this conflicts with the name field being non-nullable)

Given that RawOrganization.name is non-nullable (line 102-107), name should be required in create_or_update.

Copilot · 2026-02-02T11:02:27Z

organization/models.py

+        if self.range:
+            return f"{self.organization} - {self.role} ({self.range})"
+        return f"{self.organization} - {self.role}"


The BaseOrganizationRole class inherits from BaseDateRange and includes an organization ForeignKey and role field. However, the __str__ method (lines 1068-1071) references self.range which comes from BaseDateRange, but it doesn't handle the case where both organization and role might be None.

If organization is None, the string representation will be "None - role_value (range)" which is not very user-friendly. Consider adding a fallback:

def __str__(self): org = self.organization or "Unknown Organization" role = self.get_role_display() if self.role else "No Role" if self.range: return f"{org} - {role} ({self.range})" return f"{org} - {role}"

Suggested change

if self.range:

return f"{self.organization} - {self.role} ({self.range})"

return f"{self.organization} - {self.role}"

org = self.organization or _("Unknown Organization")

role = self.get_role_display() if self.role else _("No Role")

if self.range:

return f"{org} - {role} ({self.range})"

return f"{org} - {role}"

Copilot · 2026-02-02T11:02:27Z

organization/models.py

 import csv

 from django.apps import apps
+from django.core.exceptions import ValidationError


Import of 'ValidationError' is not used.

Suggested change

from django.core.exceptions import ValidationError

robertatakenaka added 3 commits February 1, 2026 21:57

refactor: update organization choices with SOURCE_CHOICES and ORGANIZ…

8f57a12

…ATION_ROLES

feat: redesign organization models with raw data support and hierarch…

23ff232

…ical levels

Adiciona migracao de organization

dbf0859

Copilot AI review requested due to automatic review settings February 2, 2026 01:01

Copilot started reviewing on behalf of robertatakenaka February 2, 2026 01:01 View session

robertatakenaka changed the title ~~Remodela organization parte 3x~~ Cria RawOrganization e refatora Organization Feb 2, 2026

Copilot AI reviewed Feb 2, 2026

View reviewed changes

robertatakenaka requested a review from Copilot February 2, 2026 10:54

Copilot started reviewing on behalf of robertatakenaka February 2, 2026 10:54 View session

Copilot AI reviewed Feb 2, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cria RawOrganization e refatora Organization #1268

Cria RawOrganization e refatora Organization #1268

robertatakenaka commented Feb 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

-        unique_together = [("name", "location", "external_id", "source")]  # NOVO
+        constraints = [
+            models.UniqueConstraint(
+                fields=["name", "location", "external_id", "source"],
+                condition=Q(external_id__isnull=False, source__isnull=False),
+                name="uniq_organization_name_location_external_source_not_null",
+            ),
+        ]

	unique_together = [("name", "location", "external_id", "source")] # NOVO
	unique_together = [("name", "location", "external_id", "source")]

Cria RawOrganization e refatora Organization #1268

Are you sure you want to change the base?

Cria RawOrganization e refatora Organization #1268

Conversation

robertatakenaka commented Feb 2, 2026

O que esse PR faz?

Onde a revisão poderia começar?

Como este poderia ser testado manualmente?

Algum cenário de contexto que queira dar?

Screenshots

Quais são tickets relevantes?

Referências

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant