Use folder name as id when having id mismatch to allow deletion

[Boy132]

No description provided.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4eb0dc5 and 403e880.

📒 Files selected for processing (1)

• app/Models/Plugin.php

🚧 Files skipped from review as they are similar to previous changes (1)

• app/Models/Plugin.php

---

📝 Walkthrough

Walkthrough

Changed how plugin folder names are lowercased and how plugin IDs are validated/assigned: the folder name is lowercased only during comparison (not mutated in-loop), ID validation compares against the lowercased folder, and error handling sets the entry ID to the folder on a PluginIdMismatchException, otherwise to data['id'] or a UUID.

Changes

Cohort / File(s)	Summary
Plugin ID Validation Refactor app/Models/Plugin.php	Removed in-loop mutation of the $plugin folder variable; comparisons now use Str::lower($plugin) inline. Validation now compares data['id'] to the lowercased folder. Error path sets 'id' to the folder name when a PluginIdMismatchException occurs, otherwise falls back to data['id'] or a generated UUID.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	No pull request description was provided by the author, making it impossible to assess relevance to the changeset.	Add a description explaining the purpose of the change, the problem it solves, and any relevant context about id mismatch handling in plugins.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: using the folder name as id when a plugin id mismatch occurs to enable deletion.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

app/Models/Plugin.php (1)

160-162: Using raw $plugin (not lowercased) is correct here — but the UUID fallback is unreachable for the mismatch branch.

The raw folder name from File::basename() is the right choice for the PluginIdMismatchException path: plugin_path($this->id, ...) needs the exact folder name for deletion to succeed on case-sensitive filesystems. Using Str::lower($plugin) here would break deletion when the folder has mixed-case naming.

The ?? Str::uuid() null-coalescing guard is unreachable in the PluginIdMismatchException branch since File::basename($directory) always returns a non-null string. It only meaningfully protects the $data['id'] branch (where the JSON had a null id field). The ternary structure makes this slightly implicit — a comment or a dedicated fallback per branch would improve clarity, but it's not wrong.

✨ Optional: make fallback intent explicit per branch

-'id' => ($exception instanceof PluginIdMismatchException ? $plugin : $data['id']) ?? Str::uuid(),
+'id' => $exception instanceof PluginIdMismatchException
+    ? $plugin                           // use exact folder name to allow deletion
+    : ($data['id'] ?? Str::uuid()),     // UUID fallback only needed when id was null in plugin.json

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/Models/Plugin.php` around lines 160 - 162, The code currently uses the
raw $plugin value (correct) for the PluginIdMismatchException branch but
combines both branches with a single null-coalescing fallback (?? Str::uuid()),
which is misleading because File::basename($directory) never returns null so the
UUID fallback is unreachable for the mismatch branch; update the assignment so
the branches are explicit: when ($exception instanceof
PluginIdMismatchException) set 'id' => $plugin (preserving case) and when not,
set 'id' => $data['id'] ?? Str::uuid(); alternatively add a short comment near
PluginIdMismatchException explaining that File::basename guarantees a non-null
folder name and that the UUID fallback only applies to the $data['id'] branch
(refer to PluginIdMismatchException, $plugin, $data['id'], Str::uuid(),
File::basename, and plugin_path to locate the code).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@app/Models/Plugin.php`:
- Around line 160-162: The code currently uses the raw $plugin value (correct)
for the PluginIdMismatchException branch but combines both branches with a
single null-coalescing fallback (?? Str::uuid()), which is misleading because
File::basename($directory) never returns null so the UUID fallback is
unreachable for the mismatch branch; update the assignment so the branches are
explicit: when ($exception instanceof PluginIdMismatchException) set 'id' =>
$plugin (preserving case) and when not, set 'id' => $data['id'] ?? Str::uuid();
alternatively add a short comment near PluginIdMismatchException explaining that
File::basename guarantees a non-null folder name and that the UUID fallback only
applies to the $data['id'] branch (refer to PluginIdMismatchException, $plugin,
$data['id'], Str::uuid(), File::basename, and plugin_path to locate the code).

---

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 593f209 and 4eb0dc5.

📒 Files selected for processing (1)

• app/Models/Plugin.php