Skip to content

fix(createCollectionsForAllMembers): skip existing collections, fix N+1 API calls, fix unlock branch - #48

Open
mosulcoder wants to merge 1 commit into
bitwarden-labs:mainfrom
mosulcoder:fix/createCollectionsForAllMembers-skip-existing
Open

fix(createCollectionsForAllMembers): skip existing collections, fix N+1 API calls, fix unlock branch#48
mosulcoder wants to merge 1 commit into
bitwarden-labs:mainfrom
mosulcoder:fix/createCollectionsForAllMembers-skip-existing

Conversation

@mosulcoder

@mosulcoder mosulcoder commented Jul 2, 2026

Copy link
Copy Markdown

Summary

This PR improves createCollectionsForAllMembers.sh with several reliability and performance fixes.


Behavior change: skip existing collections instead of updating

Previously, when a collection already existed for a member (matched by externalId), the script would update its name. This PR changes that behavior: if a collection with the matching name already exists inside the parent collection, the script skips that member and moves on to the next one.

# Before — would update the collection name
Updating collection '7ba76300-...' name to 'Users/Sachin Malik' while preserving all attributes.

# After — skips and moves on
Collection 'Users/Sachin Malik' already exists (ID: 7ba76300-...). Skipping member 'Sachin Malik'.

Performance: eliminate N+1 API calls

The previous script called bw list org-collections once per member inside the loop. With ~10 members that was ~10 redundant API calls (~4s each), adding ~40s of unnecessary wait time per run.

The collection list is now fetched once before the loop and filtered in-memory with jq.


Fix: broken unlock branch

The previous unlock logic had an inverted condition where the else branch (triggered when the vault was already unlocked) called bw unlock --raw without supplying credentials, which would hang waiting for stdin. Both paths now consistently use printf "%s" "$password" | bw unlock --raw (which is idempotent).


Additional improvements

  • Loop runs in current shell: Changed echo | while read pipeline to process substitution while read; done < <(...) so counters and set -e behave correctly
  • Added bw sync before fetching to ensure the CLI cache is fresh
  • Removed unused curl from the dependency check (it was listed but never called)
  • Empty name guard: fallback to member ID if all name transformations produce an empty string
  • Run summary: final log line now shows Created: X | Skipped (already existed): Y

…+1 calls, fix unlock branch

- Skip instead of update: when a collection already exists inside the
  parent collection for a member, the script now logs a message and
  moves on to the next member rather than updating it

- Eliminate N+1 API calls: fetch all org-collections once before the
  loop and filter in-memory with jq, instead of calling bw list
  org-collections once per member (~4s x N wasted API calls per run)

- Fix broken unlock branch: remove the if/else unlock block where the
  'already unlocked' else branch called bw unlock without providing
  credentials (would hang on stdin). Always supply password via printf;
  bw unlock is idempotent

- Switch loop to process substitution: change 'echo | while read' to
  'while read; done < <(...)' so the loop runs in the current shell,
  fixing counter variable scoping and set -e reliability

- Add bw sync before fetching to ensure the CLI cache is fresh

- Remove unused curl from dependency check

- Add empty member name fallback to member ID

- Add created/skipped summary count at end of run
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