-
Notifications
You must be signed in to change notification settings - Fork 29
MC Modrinth: Implement mod/plugin management with update and uninstall capabilities #96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
MC Modrinth: Implement mod/plugin management with update and uninstall capabilities #96
Conversation
Introduces update and uninstall actions for Minecraft mods/plugins, with UI enhancements for status tracking and confirmation dialogs. Adds persistent metadata management via a .modrinth-metadata.json file to track installed mods, versions, and installation dates. Updates translations, documentation, and bumps plugin version to 1.1.0.
Deleted 'download' related keys from both German and English language files as they are no longer needed.
📝 WalkthroughWalkthroughAdds install/update/uninstall mod management with persistent Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Page as MinecraftModrinthProjectPage
participant Service as MinecraftModrinthService
participant API as Modrinth API
participant FileRepo as DaemonFileRepository
participant Server as Server
Note over User,Server: Install Mod Flow
User->>Page: Click Install
Page->>API: Fetch latest version & files
API-->>Page: Version & file info
Page->>Page: Select primary file & validate filename
Page->>API: Download file content
API-->>Page: File content
Page->>FileRepo: Save file to server
FileRepo->>Server: Persist mod file
Page->>Service: Save mod metadata (.modrinth-metadata.json)
Service->>FileRepo: Write metadata
Page->>Page: Clear caches & emit notification
Page-->>User: Install success
Note over User,Server: Update Mod Flow
User->>Page: Click Update
Page->>Service: Load installed metadata
Page->>API: Fetch available versions
API-->>Page: Versions list
Page->>Page: Detect newer version, confirm with user
User-->>Page: Approve
Page->>API: Download new file
API-->>Page: New file
Page->>FileRepo: Save new file and delete old file
FileRepo->>Server: Replace files
Page->>Service: Update metadata
Page-->>User: Update success
Note over User,Server: Uninstall Mod Flow
User->>Page: Click Uninstall
Page->>User: Confirm uninstall
User-->>Page: Approve
Page->>Service: Remove metadata entry
Service->>FileRepo: Update metadata file
Page->>FileRepo: Delete mod file from server
Page->>Page: Clear caches & emit notification
Page-->>User: Uninstall success
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🧰 Additional context used🧬 Code graph analysis (2)minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php (2)
minecraft-modrinth/src/Services/MinecraftModrinthService.php (3)
🔇 Additional comments (16)
✏️ Tip: You can disable this entire section by setting 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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php (1)
18-20:⚠️ Potential issue | 🟠 MajorRemove unused
Sectionimport to fix Pint failure.The lint failure (
no_unused_imports) points to this unused import.🧹 Suggested fix
-use Filament\Schemas\Components\Section;
🤖 Fix all issues with AI agents
In
`@minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php`:
- Around line 213-232: The primaryFile['filename'] from Modrinth must be
validated/sanitized before using it in fileRepository->pull and
MinecraftModrinth::saveModMetadata; call the existing validateFilename() (or
equivalent sanitizer) on $primaryFile['filename'] inside the block that gets
$primaryFile (around getPrimaryFile(...) and before
$fileRepository->setServer(...)->pull(...)) and use the sanitized filename for
both the pull destination and the saved metadata (replace uses of
$primaryFile['filename'] with the sanitized variable); apply the same change to
the similar logic in the other occurrence referenced (lines ~319-351) so no
unvalidated filename is persisted or used for file paths.
In `@minecraft-modrinth/src/Services/MinecraftModrinthService.php`:
- Around line 188-196: The code checks the result of
$fileRepository->setServer($server)->putContent(...) against false, but
DaemonFileRepository::putContent() returns an Illuminate\Http\Client\Response,
so replace the boolean check with the response failure check: call ->failed() on
the $result (i.e. if ($result->failed()) return false;). Update both places
where metadata is written (the blocks that call getMetadataFilePath($server) and
then putContent via $fileRepository->setServer(...)->putContent) to use
$result->failed() instead of === false.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
minecraft-modrinth/README.mdminecraft-modrinth/lang/de/strings.phpminecraft-modrinth/lang/en/strings.phpminecraft-modrinth/plugin.jsonminecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.phpminecraft-modrinth/src/Services/MinecraftModrinthService.php
🧰 Additional context used
🧬 Code graph analysis (1)
minecraft-modrinth/src/Services/MinecraftModrinthService.php (2)
minecraft-modrinth/src/Enums/ModrinthProjectType.php (2)
fromServer(29-45)getFolder(21-27)minecraft-modrinth/src/Enums/MinecraftLoader.php (1)
fromServer(23-30)
🪛 GitHub Actions: Lint
minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php
[error] 1-1: Pint: 'no_unused_imports' issue detected. Remove unused imports in MinecraftModrinthProjectPage.php.
🪛 GitHub Check: PHPStan (8.2)
minecraft-modrinth/src/Services/MinecraftModrinthService.php
[failure] 224-224:
Strict comparison using === between Illuminate\Http\Client\Response and false will always evaluate to false.
[failure] 194-194:
Strict comparison using === between Illuminate\Http\Client\Response and false will always evaluate to false.
🪛 GitHub Check: PHPStan (8.3)
minecraft-modrinth/src/Services/MinecraftModrinthService.php
[failure] 224-224:
Strict comparison using === between Illuminate\Http\Client\Response and false will always evaluate to false.
[failure] 194-194:
Strict comparison using === between Illuminate\Http\Client\Response and false will always evaluate to false.
🪛 GitHub Check: PHPStan (8.4)
minecraft-modrinth/src/Services/MinecraftModrinthService.php
[failure] 224-224:
Strict comparison using === between Illuminate\Http\Client\Response and false will always evaluate to false.
[failure] 194-194:
Strict comparison using === between Illuminate\Http\Client\Response and false will always evaluate to false.
🪛 GitHub Check: PHPStan (8.5)
minecraft-modrinth/src/Services/MinecraftModrinthService.php
[failure] 224-224:
Strict comparison using === between Illuminate\Http\Client\Response and false will always evaluate to false.
[failure] 194-194:
Strict comparison using === between Illuminate\Http\Client\Response and false will always evaluate to false.
🔇 Additional comments (7)
minecraft-modrinth/plugin.json (1)
5-6: Version/description update matches the new feature set.minecraft-modrinth/lang/en/strings.php (1)
38-63: Strings align cleanly with the new install/update/uninstall flow.minecraft-modrinth/lang/de/strings.php (1)
38-63: German translations are consistent with the new action set.minecraft-modrinth/src/Services/MinecraftModrinthService.php (2)
105-154: Metadata file path + validation flow looks solid.
236-277: Helper accessors + update check are clear and well-scoped.minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php (1)
36-148: Helper methods for metadata caching and filename validation look good.minecraft-modrinth/README.md (1)
3-41: Docs clearly reflect the new install/update/uninstall lifecycle.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php
Show resolved
Hide resolved
|
I also thought about adding a filter to only show installed mods/plugins, but I'm not sure whether I should just add the “Installed only” filter or also add “Updates available.” |
Added filename validation to prevent path traversal when handling files from the Modrinth API. Updated file save logic to check for failed responses instead of boolean false, improving reliability and security.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php (1)
20-20:⚠️ Potential issue | 🟡 MinorRemove unused import
Section.The pipeline failure indicates an unused import. The
Filament\Schemas\Components\Sectionimport is not used in this file.🧹 Proposed fix
use Filament\Schemas\Components\Grid; -use Filament\Schemas\Components\Section; use Filament\Schemas\Schema;
🤖 Fix all issues with AI agents
In `@minecraft-modrinth/src/Services/MinecraftModrinthService.php`:
- Around line 105-110: getMetadataFilePath currently calls
ModrinthProjectType::fromServer($server)->getFolder() without checking for null,
which can cause a fatal error when fromServer returns null; update
getMetadataFilePath in MinecraftModrinthService.php to handle the null case by
first assigning $type = ModrinthProjectType::fromServer($server), then if $type
is null either throw a clear exception (e.g., InvalidArgumentException with a
message referencing the Server) or return a sensible default path, otherwise
call $type->getFolder() to build and return the metadata path.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.phpminecraft-modrinth/src/Services/MinecraftModrinthService.php
🧰 Additional context used
🧬 Code graph analysis (2)
minecraft-modrinth/src/Services/MinecraftModrinthService.php (2)
minecraft-modrinth/src/Enums/ModrinthProjectType.php (2)
fromServer(29-45)getFolder(21-27)minecraft-modrinth/src/Enums/MinecraftLoader.php (1)
fromServer(23-30)
minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php (2)
minecraft-modrinth/src/Services/MinecraftModrinthService.php (5)
getInstalledModsMetadata(113-154)getInstalledMod(237-248)getModrinthVersions(74-103)saveModMetadata(156-204)removeModMetadata(206-234)minecraft-modrinth/src/Enums/ModrinthProjectType.php (2)
fromServer(29-45)getFolder(21-27)
🪛 GitHub Actions: Lint
minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php
[error] 1-1: Pint test failed due to no_unused_imports issue. Unused imports detected in MinecraftModrinthProjectPage.php. Step: vendor/bin/pint --test. Process exited with code 1.
🔇 Additional comments (9)
minecraft-modrinth/src/Services/MinecraftModrinthService.php (4)
112-154: Well-structured metadata validation.The validation approach using
array_flipandarray_diff_keyis efficient. Good defensive coding with structure validation and safe fallback on exceptions.
156-204: Metadata save logic is correct.The approach of removing existing entries before adding the new one ensures clean updates. The
$response->failed()check correctly handles the Response object now.
250-263: LGTM!Version comparison by
version_idis reliable since Modrinth returns versions newest-first.
265-278: LGTM!Clean backward-compatible helper that extracts and normalizes filenames.
minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php (5)
79-147: Well-designed caching and validation helpers.The request-scoped caching via properties avoids redundant API/file reads. The
validateFilenamemethod properly guards against path traversal by checking for separators and usingbasename().
185-263: Install action is well-implemented.Good defensive coding: validates version structure, sanitizes filename before use, and invalidates cache on both success and failure paths. The download-before-metadata-save order is correct.
264-384: Update action handles file replacement safely.The download-first approach at line 334 prevents leaving the mod in a broken state if the download fails. Both old and new filenames are validated, and the old file is only deleted when filenames differ (line 338).
385-406: LGTM!The installed badge correctly shows for up-to-date mods and gracefully handles the case when version info is unavailable.
407-469: Uninstall action follows a safe ordering.The comment on lines 431-433 correctly explains why metadata is removed first: an orphaned file is safer than metadata pointing to a non-existent file. Filename validation prevents path traversal on the delete operation.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Added validation to ensure the server supports Modrinth mods or plugins before performing operations. Exceptions are now thrown when the project type is not available, improving error handling and preventing undefined behavior.
Deleted the import of Filament\Schemas\Components\Section from MinecraftModrinthProjectPage.php as it was not used in the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php (1)
18-21:⚠️ Potential issue | 🟡 MinorRemove unused
Sectionimport to satisfy lint.
The lint job reportsno_unused_imports, andFilament\Schemas\Components\Sectionisn’t referenced here.🧹 Remove unused import
-use Filament\Schemas\Components\Section;
🤖 Fix all issues with AI agents
In
`@minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php`:
- Around line 187-270: The install action can leave a downloaded JAR on disk if
MinecraftModrinth::saveModMetadata(...) returns false; after calling
$fileRepository->setServer($server)->pull(...), ensure you delete the downloaded
file on metadata-save failure (use the same $fileRepository with $server and the
target folder from $type->getFolder() and the validated $safeFilename) before
throwing/returning an error so the filesystem and UI stay consistent; update the
closure that calls DaemonFileRepository->pull and
MinecraftModrinth::saveModMetadata to perform this rollback cleanup when save
fails and then surface the failure to trigger the existing catch/notification
path.
- Around line 271-395: The current update action deletes the old file after
downloading the new file but before saving metadata, which can leave metadata
pointing to a deleted file if MinecraftModrinth::saveModMetadata fails; change
the flow in the Action::make('update')->action(...) closure to: 1) download the
new file with $fileRepository->pull as now, 2) call
MinecraftModrinth::saveModMetadata(...) immediately to update metadata to the
new filename/version, 3) only after a successful save delete the old file via
Http::daemon(...)->post(...), and 4) if save fails, rollback by deleting the
newly downloaded file (use the same $folder and $safeNewFilename validated via
validateFilename) and rethrow/handle the exception; also ensure
installedModsMetadata is invalidated consistently after successful update or on
rollback failure.
In `@minecraft-modrinth/src/Services/MinecraftModrinthService.php`:
- Around line 257-270: The isUpdateAvailable method assumes
$availableVersions[0] is newest; instead sort $availableVersions by the version
published timestamp (e.g. the version's "published" or "date_published" field)
in descending order before picking the latest, then compare
$installedMod['version_id'] to the sorted first element's 'id'; implement the
sort inside isUpdateAvailable (e.g. with usort) so the latest version is
deterministically selected.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.phpminecraft-modrinth/src/Services/MinecraftModrinthService.php
🧰 Additional context used
🧬 Code graph analysis (1)
minecraft-modrinth/src/Services/MinecraftModrinthService.php (2)
minecraft-modrinth/src/Enums/ModrinthProjectType.php (2)
fromServer(29-45)getFolder(21-27)minecraft-modrinth/src/Enums/MinecraftLoader.php (1)
fromServer(23-30)
🪛 GitHub Actions: Lint
minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php
[error] 1-1: vendor/bin/pint --test: no_unused_imports violation in MinecraftModrinthProjectPage.php.
🔇 Additional comments (17)
minecraft-modrinth/src/Services/MinecraftModrinthService.php (6)
105-116: Good guard for unsupported server types.
Clear exception prevents null dereferences and gives a helpful error.
119-160: Robust metadata validation.
Required-key filtering plus safe fallback keeps invalid metadata out of the UI.
163-210: Metadata write flow is clean.
Filtering existing entries before appending makes updates deterministic.
213-240: Clean removal path.
243-255: Straightforward lookup.
272-285: Back-compat filename list looks good.
Lowercasing helps keep comparisons stable.minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php (11)
36-40: Caching fields look good.
56-64: Graceful fallback label.
81-94: Lazy-load metadata cache is solid.
96-108: Simple lookup is fine.
110-119: Per-project version cache is fine.
122-135: Primary-file selection is clear.
137-149: Good filename sanitization guard.
397-418: Installed-state action is clear.
419-485: Uninstall flow is consistent.
490-507: Header action guard is solid.
509-549: Installed-count display update looks good.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php
Show resolved
Hide resolved
minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php`:
- Around line 247-248: When clearing the installed mods metadata, also clear the
versions cache to avoid stale state: wherever you set
$this->installedModsMetadata = null (e.g. the occurrences at the shown
locations), also set $this->versionsCache = null in the same block; ensure every
success/failure cache reset that touches $this->installedModsMetadata includes
$this->versionsCache = null so both caches are reset together.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php
🧰 Additional context used
🧬 Code graph analysis (1)
minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php (4)
minecraft-modrinth/src/Enums/ModrinthProjectType.php (3)
fromServer(29-45)getLabel(13-19)getFolder(21-27)minecraft-modrinth/src/Enums/MinecraftLoader.php (2)
fromServer(23-30)getLabel(18-21)minecraft-modrinth/src/Services/MinecraftModrinthService.php (4)
getInstalledModsMetadata(120-161)getInstalledMod(244-255)saveModMetadata(163-211)removeModMetadata(213-241)minecraft-modrinth/src/Facades/MinecraftModrinth.php (1)
MinecraftModrinth(19-25)
🔇 Additional comments (3)
minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php (3)
28-148: Nice helper/cache layer with filename sanitization.
Clear separation of metadata lookup, version caching, and filename validation keeps the action handlers much cleaner and safer.
396-417: Installed badge logic looks solid.
The visibility rules map cleanly to “installed + latest” vs “installed but outdated.”
494-547: Server-type guard additions are a good safety net.
Returning empty header actions and “unknown” states when unsupported avoids null issues and confusing UI.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php
Show resolved
Hide resolved
Adds rollback logic to delete downloaded files if saving mod metadata fails during install or update, ensuring consistency. Moves old file deletion after successful metadata save during updates. Ensures UI cache is invalidated after install, update, or uninstall actions. Adds defensive sorting of Modrinth versions by publish date to guarantee latest version is first.
Introduces a new 'Installed' view for Modrinth projects, allowing users to filter and display only installed mods. Adds logic to fetch installed mods' data in bulk from the Modrinth API, gracefully handles unavailable mods by displaying a message, and updates language files for new UI labels. Also ensures author metadata is stored and displayed where available.
Updated type annotations and logic in both MinecraftModrinthProjectPage and MinecraftModrinthService to support an optional 'author' field in installed mod metadata arrays. This allows mod author information to be stored and retrieved when available.
Overview
This PR adds comprehensive mod lifecycle management to the Minecraft Modrinth plugin. Currently, users can only install mods but have no way to update or remove them through the interface. This creates a frustrating user experience where they need to manually manage files outside the plugin.
Changes
Core Functionality
I've implemented update and uninstall capabilities for both mods and plugins. When users view their installed items, they'll now see action buttons that allow them to update to the latest version or remove items entirely. Both operations include confirmation modals to prevent accidental actions.
Metadata File
To make updates and uninstalls possible, I introduced a metadata tracking system using
.modrinth-metadata.json. This file stores essential information about each installed mod (version, filename, installation date), which lets us determine when updates are available and ensures clean uninstallationLocalization & Documentation
I've added the necessary translation strings for English and German to support the new UI elements, and updated the README to document the new features
Implementation Notes
The metadata file approach was chosen over database storage to keep the plugin lightweight and make the data easily portable with the mod directory. The file is updated on each install/uninstall operation and read when checking for available updates.
Tested
Version
Bumped to
1.1.0to reflect the new feature set.Fixes Issues
Closes #62
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.