Improve MaterialList loading

[BacLuc]

Replace requests for each category/materialnode with a request to the collection with a filter.

[BacLuc]

@CodeRabbit review this pr

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

@BacLuc I'll review PR #7657 for you. Let me gather the necessary information about the pull request first.

---

🏁 Script executed:

#!/bin/bash
# Get basic information about the PR
gh pr view 7657 --repo ecamp/ecamp3 --json title,body,author,state,files,additions,deletions,commits

# Get the list of changed files
gh pr diff 7657 --repo ecamp/ecamp3 --name-only

# Get the actual diff to see what changes were made
gh pr diff 7657 --repo ecamp/ecamp3

Length of output: 4414

---

PR Review: Improve MaterialList loading

@BacLuc Great work on optimizing the material loading performance! This PR effectively addresses the performance concern by batching requests instead of making individual ones. Here's my detailed review:

✅ Positive Changes

1. Performance Optimization: The core objective is well-executed - replacing multiple individual requests with batched requests will significantly improve loading times.

2. Early Request Initiation: Starting the material nodes loading in mounted() is a smart optimization that allows the request to begin early in the component lifecycle.

3. Consistent Promise Management: Using Promise.all() to coordinate multiple async operations is the right approach.

⚠️ Areas of Concern

1. Promise Assignment Logic Issue

// In mounted()
this.loadMaterialNodesPromise = this.period
  ? this.loadMaterialNodes()
  : Promise.resolve()

// But inside loadMaterialNodes()
this.loadMaterialNodesPromise = await this.api.get().contentNodes({...}).$loadItems()

This creates confusion - you're assigning the promise twice. Consider this fix:

async loadMaterialNodes() {
  await this.api.get().contentTypes().$loadItems()
  const contentTypeMaterial = this.api
    .get()
    .contentTypes()
    .items.find((ct) => ct.name === 'Material')
  if (!contentTypeMaterial) {
    throw new Error('Material content type not found')
  }
  return await this.api
    .get()
    .contentNodes({
      camp: this.camp._meta.self,
      contentType: contentTypeMaterial._meta.self,
    })
    .$loadItems()
}

2. Error Handling

The loadMaterialNodes() method throws an error if the Material content type isn't found, but this error isn't caught anywhere. Consider adding error handling in the mounted() method or making the method more resilient.

3. Breaking API Change

// Before
:activity-promise="findOneActivityByContentNode(item.entityObject.materialNode())"

// After  
:activity-promise="findOneActivityByContentNode(item.entityObject)"

The parameter change from contentNode to entityObject is a breaking change. While it works for this internal usage, ensure no other components depend on the old signature.

4. Race Condition Potential

If findOneActivityByContentNode() is called before mounted() completes, loadMaterialNodesPromise might be null. Consider initializing it in the data() section:

data() {
  return {
    // ...
    loadMaterialNodesPromise: Promise.resolve(),
  }
}

🔍 Minor Suggestions

1. Method Naming: Consider renaming loadMaterialNodes() to initializeMaterialNodes() to better reflect its purpose as an initialization method.

2. Documentation: Adding JSDoc comments to the new methods would help future maintainers understand the optimization strategy.

📊 Performance Impact

The changes should provide measurable performance improvements, especially for camps with many material categories. The batched loading approach is architecturally sound.

Overall, this is a solid performance optimization with a few implementation details that could be refined. The core approach is excellent! 🚀

[github-actions]

⛔ Feature branch deployment currently inactive.

If the PR is still open, you can add the deploy! label to this PR to trigger a feature branch deployment.

[BacLuc]

The resulting request to the content_nodes
https://app.ecamp3.ch/api/content_nodes.jsonhal?camp=camps/{campid}&contentType=/api/content_types/3ef17bd1df72
was performing very bad at times, but then it seemed ok again...?

[carlobeltrame]

Clever!