Skip to content

Check reference in validator#539

Merged
vkrasnovyd merged 6 commits into
OpenSlides:mainfrom
vkrasnovyd:check-reference-in-validator
Jul 9, 2026
Merged

Check reference in validator#539
vkrasnovyd merged 6 commits into
OpenSlides:mainfrom
vkrasnovyd:check-reference-in-validator

Conversation

@vkrasnovyd

Copy link
Copy Markdown
Contributor

Resolves #415 and makes validator fail when reference attribute (or it absence) can cause inconsistencies in schema.

Allowed combinations that result in correct schema:

  • relation (r) - relation (x)
  • generic-relation (r) - relation (x)
  • relation (r) - relation-list (r | x) -> reference for relation-list is ignored, generated schema is the same
  • generic-relation (r) - relation-list (r | x) -> reference for relation-list is ignored, generated schema is the same
  • relation-list (x) - relation-list (x)
  • generic-relation-list (x) - relation-list (x)

Comment thread dev/src/validate.py
Comment thread dev/src/validate.py Outdated
Comment on lines +729 to +732
if required and not from_field.get("reference"):
return f"Relational collectionfield {from_collectionfield} must have 'reference' attribute."

return None

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What if both sides are required?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Result is the same: the fields are being generated in both tables instead of table + view. required in this case changes only whether the NOT NULL constraint exists.

Similar for the other field combinations.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

But in this case required refers not to the field attribute but to whether the reference attribute is required for this side of the relation. If it is required but not set, the error will be added to self.errors.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ah, then the variable should probably get a more verbose name.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment thread dev/src/validate.py
@vkrasnovyd

Copy link
Copy Markdown
Contributor Author

Working notes:

Correct schema is produced

  • relation (r) - relation (x)
  • generic-relation (r) - relation (x)
  • relation (r) - relation-list (r | x) -> reference for relation-list is ignored, generated schema is the same
  • generic-relation (r) - relation-list (r | x) -> reference for relation-list is ignored, generated schema is the same
  • relation-list (x) - relation-list (x)
  • generic-relation-list (x) - relation-list (x)

Implemented in schema generator, but generate invalid schema

These relations are implemented in schema generator (before this PR), but the produced schema is not correct. Should become not allowed in both collections validator and schema generator.

Field generated in both tables instead of table-view:

  • relation (r) - relation (r)
  • generic-relation (r) - relation (r)

2 nm-tables get generated:

  • relation-list (r) - relation-list (r) - without required fields

Not implemented in schema generator

No fields get generated, error is added to the end of the generated schema file. Should also make validator scream.

  • relation - generic-relation-list

  • generic-relation - generic-relation

  • generic-relation - generic-relation-list

  • generic-relation-list - generic-relation-list

  • relation (x) - relation-list (x)

  • relation (x) - relation-list (r)

  • generic-relation (x) - relation (x)

  • generic-relation (x) - relation-list (x)

  • generic-relation (x) - relation-list (r)

  • relation-list (r) - relation-list (r) - when any of the sides is required

  • relation-list (r) - relation-list (x) (reference on any of the sides) - when any of the sides is required

  • generic-relation-list (r) - relation-list (r)

  • generic-relation-list (x) - relation-list (r)

  • generic-relation-list (r) - relation-list (x)

@luisa-beerboom luisa-beerboom left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have one minor thing, then it's good

Comment thread dev/src/validate.py
Comment thread dev/src/validate.py Outdated
Comment on lines +729 to +732
if required and not from_field.get("reference"):
return f"Relational collectionfield {from_collectionfield} must have 'reference' attribute."

return None

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ah, then the variable should probably get a more verbose name.

@vkrasnovyd vkrasnovyd requested a review from luisa-beerboom July 9, 2026 08:03
@vkrasnovyd vkrasnovyd merged commit 31a750a into OpenSlides:main Jul 9, 2026
2 checks passed
@vkrasnovyd vkrasnovyd deleted the check-reference-in-validator branch July 9, 2026 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Don't generate 1r:1r, 1r:1GrR and nr:nr relations

2 participants