491 all projects

[cubap]

This pull request refactors the authentication and project-loading logic in the ProjectsListNavigation component to improve error handling and event-driven updates. The changes focus on separating concerns between authentication and project loading, and ensuring that errors are communicated consistently to the user.

Authentication and project loading improvements:

• The component now listens for the tpen-authenticated event to trigger project fetching, handling errors with a toast notification and updating the UI if fetching fails.
• Project data assignment is moved to a separate listener for the tpen-user-projects-loaded event, ensuring that the UI updates only when projects are successfully loaded.

Error handling enhancements:

• Error handling is unified for both authentication and project loading, with consistent toast notifications and UI updates when errors occur.

[github-actions]

🚀 Deployed on https://6998cd03554bb864a7cb7362--tpen-interfaces-preview.netlify.app

[thehabes]

Tested locally and do not experience any issues. I see the projects I expect to see, they are not duplicated, they correctly show or don't show the cog, all focus and activate actions (keyboard and mouse) work as expected.

Below are some non-blocking things found during /static-review. Decide whether they have merit or should be addressed.

Static Review Comments

Branch: 491-all-projects
Review Date: 2026-02-20
Reviewer: Claude Code Static Reviewer

A few issues are worth addressing before merge.

Category	Issues Found
🔴 Critical	0
🟠 Major	1
🟡 Minor	1
🔵 Suggestions	1

Positive Observations

Before diving into issues, several things are done well here:

1. Bug fix — incorrect dispatch() call: The old code passed a CustomEvent object to eventDispatcher.dispatch(), which expects a string. This meant toast notifications on error were silently broken (the event name would coerce to "[object CustomEvent]"). The new dispatch('tpen-toast', { ... }) is correct.
2. Race condition fix: Moving listener registration before TPEN.attachAuthentication(this) ensures the tpen-authenticated event can't fire before the listener is attached.
3. Clean separation: Auth triggers fetch → fetch dispatches event → event updates UI. This is idiomatic event-driven design and avoids the component duplicating the work TPEN.getUserProjects() already does internally.
4. DOM performance: Replacing innerHTML += (which re-parses the entire list on every iteration) with projectItems.push() + a single innerHTML = join('') is a meaningful improvement for users with many projects.

---

Major Issues 🟠

Issue 1: Ineffective try/catch in tpen-user-projects-loaded handler

File: components/projects/list-navigation.js:134-145
Category: Logic Error / Dead Code

Problem:
The synchronous try/catch wrapping this.projects = TPEN.userProjects provides almost no real error coverage. TPEN.userProjects is a simple getter (return this.#userProjects) that won't throw. The setter assigns and calls this.updateList() — but updateList() is async, so it returns a Promise. Any error inside updateList() (e.g., a failed Project.fetch() in the loop) becomes an unhandled Promise rejection, not a synchronous throw. The catch block here will never execute in practice, giving a false sense of safety.

Current Code:

this.cleanup.onEvent(TPEN.eventDispatcher, "tpen-user-projects-loaded", () => {
    try {
        this.projects = TPEN.userProjects
    } catch (error) {
        const status = error.status ?? 500
        const text = error.statusText ?? error.message ?? "Internal Error"
        TPEN.eventDispatcher.dispatch('tpen-toast', {
            message: `Error fetching projects: ${text}`,
            status: status
        })
        this.shadowRoot.getElementById('projectsListView').innerHTML = `No projects found`
    }
})

Suggested Fix:
Either remove the try/catch entirely (since it can't trigger) or, if you want safety around updateList, handle the async error:

this.cleanup.onEvent(TPEN.eventDispatcher, "tpen-user-projects-loaded", () => {
    this.projects = TPEN.userProjects
})

If you want to guard updateList() failures, add a .catch() inside the setter or at the updateList call site:

set projects(projects) {
    this.#projects = projects
    this.updateList().catch((error) => {
        console.warn('Failed to update project list:', error)
        const list = this.shadowRoot.getElementById('projectsListView')
        if (list) list.innerHTML = `No projects found`
    })
}

How to Verify:

1. Temporarily add throw new Error('test') as the first line of updateList().
2. With the current code, observe the error is an unhandled rejection (not caught by the try/catch).
3. With the fix, confirm the error is caught and a user-friendly message is shown.

---

Minor Issues 🟡

Issue 1: Missing null guard on getElementById in error handlers

File: components/projects/list-navigation.js:129 and 144
Category: Potential Runtime Error

Problem:
Both error handlers do:

this.shadowRoot.getElementById('projectsListView').innerHTML = `No projects found`

If updateList() has already replaced this element with the welcome <section> (e.g., from a tpen-no-recent-activity event that raced ahead), getElementById returns null and this line throws an uncaught error — hiding the original error from the user.

Suggested Fix:

const list = this.shadowRoot.getElementById('projectsListView')
if (list) list.innerHTML = `No projects found`

---

Suggestions 🔵

Suggestion 1: Sequential N+1 fetches in updateList() (pre-existing)

File: components/projects/list-navigation.js:205-213
Category: Performance

Each project sequentially calls new Project(id).fetch() to check permissions. For a user with 30 projects, that's 30 waterfall network requests. This was pre-existing, but since this area was touched, it's worth noting. A parallel approach would improve perceived performance significantly:

const projectsWithPermissions = await Promise.allSettled(
    this.#projects.map(async (project) => {
        let manageLink = ''
        try {
            await new Project(project._id).fetch()
            const canEdit = CheckPermissions.checkEditAccess('PROJECT')
            manageLink = canEdit ? `<a ...>⚙</a>` : ''
        } catch (e) {
            console.warn(`Failed to check permissions for project ${project._id}:`, e)
        }
        return { project, manageLink }
    })
)

This is a larger change and could be tackled in a follow-up.