Skip to content

fix(config_guide): correct CEL and variables docs against codebase#131

Open
emjay0921 wants to merge 1 commit intov2-in-progress-3from
fix/config-guide-823-cel-variables
Open

fix(config_guide): correct CEL and variables docs against codebase#131
emjay0921 wants to merge 1 commit intov2-in-progress-3from
fix/config-guide-823-cel-variables

Conversation

@emjay0921
Copy link
Copy Markdown
Contributor

Why is this change needed?

QA review found 14 inaccuracies in the CEL and variables documentation when verified against the OpenSPP2 codebase.

How was the change implemented?

  • Fixed expression states: Draft → Active → Inactive (was Draft → Pending Approval → Published → Archived)
  • Fixed menu path: Studio → Logic Design (was Studio → Rules)
  • Removed non-existent functions: startswith(), contains(), before(), after()
  • Removed Events as aggregate target (only members, enrollments, entitlements exist)
  • Rewrote event aggregate sections to use Computed (CEL) with event functions
  • Removed undocumented where parameter and Python fallback references
  • Clarified collection predicate syntax note
  • Replaced raw Odoo model names in profiles table
  • Renamed "Variables & Indicators" to "Variables"

Related links

https://projects.acn.fr/projects/acn-eng/work_packages/823

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the OpenSPP documentation to reflect workflow changes, including renaming 'publishing' to 'activating' and updating menu navigation. It also transitions event data access from aggregate targets to dedicated CEL functions and simplifies string function recommendations. A review comment correctly identifies a need to remove the 'events' relation from the syntax guide's relations table to maintain consistency with these architectural changes.

Comment on lines +172 to +173
| `registry_individuals` | Individual registrant | `groups`, `enrollments`, `entitlements`, `events` |
| `registry_groups` | Group/household | `members`, `enrollments`, `entitlements`, `events` |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The events relation should be removed from the 'Available Relations' column to maintain consistency with the rest of the documentation changes. The PR description and other updated files (e.g., variables.md, variable_types.md) state that event data is no longer an aggregate target and should be accessed via dedicated event functions instead of collection relations.

Suggested change
| `registry_individuals` | Individual registrant | `groups`, `enrollments`, `entitlements`, `events` |
| `registry_groups` | Group/household | `members`, `enrollments`, `entitlements`, `events` |
| registry_individuals | Individual registrant | groups, enrollments, entitlements |
| registry_groups | Group/household | members, enrollments, entitlements |

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.

1 participant