support for manage access dialog - #331
SharonStrats wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Resolve the critical test-typecheck issue and the two moderate API and WebID issues.
Review effort: Lite
Findings: 1
Open (3)
What changed in this PR
Adds access-control planning APIs and WebID detection to support a manage-access dialog.
Changes:
- Adds ACL discovery, grant, and public-read planning methods.
- Re-exports access-control APIs and types.
- Adds WebID detection and dependency updates.
| File | Summary / Findings |
|---|---|
src/types.ts |
Extends ACL and resource interfaces. Critical: Update typed test doubles or make additions optional to prevent typecheck-test failure. |
src/resource/resourceLogic.ts |
Adds WebID detection. Moderate: Accept supported profile-card URL variants such as /profile/card.ttl. |
src/index.ts |
Exposes access-control APIs. Moderate: Re-export ACLContext. |
src/acl/aclLogic.ts |
Wraps ACL discovery and planning helpers. |
package.json |
Adds the access-control dependency. |
package-lock.json |
Updates dependency resolution. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
abd8524 to
f4e8c9c
Compare
Prompt: Add basic tests to cover the new acl logic functions that have been added Co-authored-by: GPT-5.4 Mini <gpt-5.4-mini@openai.com>
f4e8c9c to
ebaaeb7
Compare
| it('recognizes only profile-card.ttl WebIDs', () => { | ||
| const resourceLogic = createResourceLogic(store, aclLogic, containerLogic, typeIndexLogic) | ||
|
|
||
| expect(resourceLogic.isWebId(sym('https://alice.example.com/profile/card.ttl#me'))).toBe(true) |
There was a problem hiding this comment.
is this https://alice.example/profile/card.ttl#me a right WebID?
There was a problem hiding this comment.
I think the recommendation came because of this
solid-logic/test/helpers/dataSetup.ts
Line 4 in b0a3882
solid-logic/test/profileLogic.test.ts
Line 346 in b0a3882
I can remove it. I'm thinking that's probably the way to go as there isn't any other code like this. What are your thoughts?


Extended aclLogic to support the new manage access dialog by wiring in the necessary ACL helpers and access planning logic from @dokieli/web-access-control.
Alse added a function to resource logic to determine whether or not the resource is a webID.
Note: aclLogic will evolve as I complete the work, this is just to support the basic structure working.