Skip to content

Conversation

@bradbanister
Copy link
Contributor

No description provided.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a comprehensive authorization redesign document that replaces the current JSONB/trigger-driven authorization model with a relational, strategy-friendly design. The redesign aims to improve performance and maintainability while preserving existing ODS authorization semantics.

Key Changes:

  • Introduces a new relational data model with DocumentSubject and SubjectEdOrg tables to replace JSONB authorization arrays and pathway-specific tables
  • Redesigns the EducationOrganization hierarchy using a clean adjacency model with recursive functions
  • Integrates authorization filtering with the DocumentIndex query/indexing design for efficient authorized data access

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

used in the write pipeline over `EdfiDoc` and the resource schema.
3. Invoke `ResourceAuthorizationHandler` for the `ExistingData` phase with the
appropriate strategies and the in‑memory `DocumentSecurityElements`.
3. Validators use `IAuthorizationRepository` (which now reads `SubjectEdOrg`)
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

Duplicate step numbering: Steps 2 and 3 both use "3." This should be corrected so that step 2 is followed by step 3, and then step 4.

Suggested change
3. Validators use `IAuthorizationRepository` (which now reads `SubjectEdOrg`)
4. Validators use `IAuthorizationRepository` (which now reads `SubjectEdOrg`)

Copilot uses AI. Check for mistakes.
- Future pathways (e.g., ProgramParticipation) can be added by introducing new
enum values.
- The combination `(SubjectType, SubjectIdentifier, Pathway)` can always be recomputed
from the corresponding relationship documents, see synchronization design is section 6.
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The phrase "synchronization design is section 6" should be "synchronization design in section 6" (missing the word "in").

Suggested change
from the corresponding relationship documents, see synchronization design is section 6.
from the corresponding relationship documents, see synchronization design in section 6.

Copilot uses AI. Check for mistakes.
### 4.3 Locking Down `SubjectType` and `Pathway` Identifiers

This design stores `SubjectType` and `Pathway` as `smallint` for compact keys
and fast joins. However, using ad-hoc numeric enums is fragile: if the numeric
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The phrase "ad-hoc" should use a space instead of a hyphen: "ad hoc" (this is the standard spelling for this Latin phrase when used as an adjective).

Suggested change
and fast joins. However, using ad-hoc numeric enums is fragile: if the numeric
and fast joins. However, using ad hoc numeric enums is fragile: if the numeric

Copilot uses AI. Check for mistakes.

---

## 12. Open Questions and Future Enhancements
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

Section numbering jumps from "9. SecurityElements and QueryFields" directly to "12. Open Questions and Future Enhancements", skipping sections 10 and 11. This should be corrected to either section 10 or the missing sections should be added.

Suggested change
## 12. Open Questions and Future Enhancements
## 10. Open Questions and Future Enhancements

Copilot uses AI. Check for mistakes.
- GIN on `QueryFields` filters by query parameters before paging.
- `DocumentSubject` + `SubjectEdOrg` joins are backed by narrow B‑tree indexes,
enabling fast existence checks.
- `totalCount` requests uses the same `WHERE` (including the `EXISTS`)
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

Subject-verb agreement error: "totalCount requests uses" should be "totalCount requests use" (plural subject requires plural verb form).

Suggested change
- `totalCount` requests uses the same `WHERE` (including the `EXISTS`)
- `totalCount` requests use the same `WHERE` (including the `EXISTS`)

Copilot uses AI. Check for mistakes.
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.

2 participants