Adiciona campos page_type, order e parent_page

[samuelveigarangel]

O que esse PR faz?

Adiciona campo page_type para tipo da página
Adiciona campo order
Adiciona campos parent_page e child_pages para referencias página pai e subpáginas.

Onde a revisão poderia começar?

pelos commits

Como este poderia ser testado manualmente?

referênciar no requirements.txt no projeto opac_5:

git+https://github.com/samuelveigarangel/opac_schema.git@issue-115#egg=opac_schema

Acessar a interface administrativa -> Pages

Algum cenário de contexto que queira dar?

N/A

Screenshots

Quais são tickets relevantes?

#115

Referências

N/A

[joffilyfe]

Oi @samuelveigarangel, tudo bem? Esse pull request possui alguma issue onde eu possa ler sobre a finalidade do PR? Obrigado.

[samuelveigarangel]

@joffilyfe #115

[Copilot]

Pull Request Overview

This PR adds hierarchical page structure capabilities to the Pages model by introducing parent-child relationships and page ordering. The changes enable pages to be organized in a tree-like structure with different page types.

• Adds page_type field with predefined choices for categorizing pages
• Introduces order field for page sequencing and parent/child page references
• Implements automatic parent-child relationship management in the save method

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

Variable assignment is incorrect. Should be parent = self.parent_page instead of parent = self.page_type. The code is trying to get the parent page object but is incorrectly assigning the page type string.

Suggested change

	parent = self.page_type
	parent = self.parent_page

[Copilot]

This save call will cause infinite recursion. When saving the parent page, it will trigger its own save method, which may lead to a stack overflow. Consider using update() method or adding a flag to prevent recursive saves.

Suggested change

	parent = self.page_type
	# evita referência a si mesmo e duplicação
	if parent.id != self.id and self not in (parent.child_pages or []):
	parent.child_pages = (parent.child_pages or []) + [self]
	super(Pages, self).save()
	parent = self.parent_page
	# evita referência a si mesmo e duplicação
	if parent.id != self.id and self not in (parent.child_pages or []):
	parent.update(push__child_pages=self)

[Copilot]

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

The condition self not in (parent.child_pages or []) will always be True because self is a Pages instance while parent.child_pages contains ObjectId references. This comparison should use self.id instead of self.

Suggested change

	if parent.id != self.id and self not in (parent.child_pages or []):
	if parent.id != self.id and self.id not in (parent.child_pages or []):

[Copilot]

The parent-child relationship update logic executes on every save operation, even when parent_page hasn't changed. Consider checking if parent_page is dirty before performing the update to avoid unnecessary database operations.

Suggested change

	if self.parent_page:
	# Só atualiza se parent_page foi alterado ou se o documento é novo
	parent_page_dirty = (
	not self.pk or # documento novo
	hasattr(self, '_changed_fields') and 'parent_page' in self._changed_fields
	)
	if self.parent_page and parent_page_dirty:

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 7 comments.

---

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

[Copilot]

The hierarchy synchronization methods (remove_self_from_old_parent, add_self_to_new_parent, remove_parent_from_removed_children, set_parent_for_current_children) each perform individual database updates and are not atomic with the main save() call. If any of these operations fail midway (e.g., a network error after the main save but before all children are updated), the parent-child relationships will be left in an inconsistent state. Additionally, there is no protection against circular references — a page could be set as its own grandparent, creating an infinite loop. Consider adding cycle detection before saving.

[Copilot]

The existing test_pages.py file has tests for the Pages model, but no tests have been added for any of the new functionality: page_type field validation, order field, parent/child hierarchy management (including get_pages_parent_and_children, remove_self_from_old_parent, add_self_to_new_parent, remove_parent_from_removed_children, set_parent_for_current_children), and the new set_slug behavior with parent pages. These behaviors include significant logic in the save() override that should be covered by tests to prevent regressions.

[Copilot]

The remove_parent_from_removed_children and set_parent_for_current_children methods iterate over children and make individual database update calls for each child. This creates an N+1 query pattern that could be slow when a page has many children. Consider using bulk update operations (e.g., Pages.objects(pk__in=...).update(...)) to reduce the number of database round-trips.

Suggested change

	for child in removed_children:
	if child.parent_page == self:
	child.update(unset__parent_page=True)
	def set_parent_for_current_children(self, current_children):
	for child in current_children:
	if child.id != self.id and child.parent_page != self:
	child.update(set__parent_page=self)
	# Filter to children that currently have this page as parent
	target_children = [
	child for child in removed_children
	if child.parent_page == self
	]
	if target_children:
	target_ids = [child.pk for child in target_children]
	Pages.objects(pk__in=target_ids).update(unset__parent_page=True)
	def set_parent_for_current_children(self, current_children):
	# Select children that should have this page set as parent
	target_children = [
	child for child in current_children
	if child.id != self.id and child.parent_page != self
	]
	if target_children:
	target_ids = [child.pk for child in target_children]
	Pages.objects(pk__in=target_ids).update(set__parent_page=self)

[Copilot]

The self not in (parent.child_pages or []) check does an in-memory comparison which may not work reliably with MongoEngine document objects — depending on whether the references are lazily loaded, the in check may compare by identity rather than by primary key, potentially leading to duplicate entries in child_pages. Consider comparing by self.pk against [c.pk for c in (parent.child_pages or [])] for a more reliable check. Alternatively, since add_to_set already prevents duplicates at the database level, this guard could be simplified to just the self-reference check.

Suggested change

	if parent.id != self.id and self not in (parent.child_pages or []):
	if parent.id != self.id:

[Copilot]

The page_type choices use "detail_about" but issue #115 specifies "main_about" as one of the predefined choices: ('main_about', 'about', 'journal', 'free'). Please verify which value is correct and align the code with the agreed-upon specification.

[Copilot]

The slug is only generated when self.slug_name is falsy (line 102). This means that if a page is renamed or its parent_page is changed, the slug will not be updated to reflect the new name or hierarchy. The slug will remain stale. Consider always recalculating the slug (or at least when name or parent_page have changed).

Suggested change

	if not self.slug_name:
	# Recalculate slug when missing, or when fields that affect it change.
	changed_fields = getattr(self, "_changed_fields", []) or []
	should_update_slug = not self.slug_name
	if "name" in changed_fields or "parent_page" in changed_fields:
	should_update_slug = True
	if should_update_slug:

[Copilot]

If self.parent_page exists but self.parent_page.slug_name is None (e.g., the parent was created before the slug logic existed, or its slug was never set), this will produce a slug like "None/child-name". Consider adding a guard to handle this case, for example by falling back to the parent's name or raising a clear error.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 7 comments.

---

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

[Copilot]

The parent_page field is queried via get_children() (line 141: Pages.objects.filter(parent_page=self)), but it is not included in the meta['indexes'] list. This will result in a full collection scan on every get_children() call. Consider adding 'parent_page' to the indexes in the meta dict.

[Copilot]

The PR title, description, and linked issue (#115) all mention adding a child_pages field (ReferenceField) to the model, but no such field is defined. Instead, a get_children() method is provided. While the method approach may be preferable from a data modeling perspective (avoiding data duplication), this is a significant discrepancy with the stated requirements. If this was intentional, the PR description and issue should be updated to reflect the design decision.

[Copilot]

set_slug has a side effect of saving the parent page (line 134) when the parent doesn't have a slug yet. This triggers a full save() on the parent (updating its updated_at, re-running validate_no_circular_reference, etc.), which is unexpected behavior from a slug generation method. Additionally, if the parent also lacks a slug and has its own parent, this could chain upward recursively through multiple saves. Consider computing the parent slug without persisting the parent, or at minimum documenting this behavior.

Suggested change

	"""Gera o slug hierárquico baseado no nome e pai."""
	if not self.parent_page:
	self.slug_name = slugify(self.name)
	return
	parent_slug = self.parent_page.slug_name
	if not parent_slug:
	self.parent_page.set_slug()
	self.parent_page.save()
	parent_slug = self.parent_page.slug_name
	self.slug_name = f"{parent_slug}/{slugify(self.name)}"
	"""Gera o slug hierárquico baseado no nome e na hierarquia de pais, sem salvar páginas pai."""
	parts = []
	current = self
	# Percorre a cadeia de pais acumulando os slugs de cada nível
	while current is not None:
	parts.append(slugify(current.name))
	current = current.parent_page
	# A lista foi construída do filho até o ancestral mais distante; invertendo obtemos a ordem correta
	parts.reverse()
	self.slug_name = "/".join(parts)

[Copilot]

The slug is only generated when self.slug_name is falsy (line 94). This means if a page's name or parent_page is changed after the initial save, the slug will not be updated to reflect the new hierarchy or name. This can lead to stale/incorrect slugs. Consider regenerating the slug whenever the page name or parent changes, and also propagating slug updates to child pages when a parent's slug changes.

[Copilot]

get_ancestors has no cycle or depth protection. If circular reference data exists in the database (e.g., inserted via direct DB manipulation bypassing the save() method's validation), this method will loop infinitely. Consider adding a max_depth guard similar to validate_no_circular_reference, or tracking visited IDs to break out of cycles.

Suggested change

	def get_ancestors(self):
	"""Retorna lista de ancestrais (pai, avô, bisavô, etc) em ordem."""
	ancestors = []
	current = self.parent_page
	while current:
	ancestors.append(current)
	current = current.parent_page
	def get_ancestors(self, max_depth=100):
	"""Retorna lista de ancestrais (pai, avô, bisavô, etc) em ordem."""
	ancestors = []
	current = self.parent_page
	depth = 0
	visited_ids = set()
	while current and depth < max_depth:
	current_id = getattr(current, "id", None)
	if current_id is not None:
	if current_id in visited_ids:
	# Detecção de ciclo: interrompe para evitar loop infinito
	break
	visited_ids.add(current_id)
	ancestors.append(current)
	current = current.parent_page
	depth += 1

[Copilot]

get_descendants is recursive with no depth limit. If the hierarchy is deep or if circular data exists in the database (bypassing save() validation), this method could cause a stack overflow. Consider adding a max_depth parameter or tracking visited IDs to prevent infinite recursion.

Suggested change

	children = self.get_children()
	for child in children:
	descendants.append(child)
	descendants.extend(child.get_descendants())
	# Use iterative traversal with cycle protection to avoid
	# potential recursion overflow or infinite loops on bad data.
	visited_ids = set()
	stack = list(self.get_children())
	while stack:
	current = stack.pop()
	current_id = getattr(current, "id", None)
	if current_id in visited_ids:
	continue
	if current_id is not None:
	visited_ids.add(current_id)
	descendants.append(current)
	# Add children of the current node to the stack
	stack.extend(Pages.objects.filter(parent_page=current))

[Copilot]

No tests were added for the new fields (page_type, order, parent_page) or new methods (validate_no_circular_reference, set_slug, get_children, get_ancestors, get_descendants). The existing tests/test_pages.py has tests for the Pages model, so this is a gap. Tests should cover at minimum: page creation with new fields, circular reference detection, hierarchical slug generation, and the tree traversal methods.