Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17fba38ce9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 'filemanager_path' => '', | ||
| 'rb_base_dir' => '', |
There was a problem hiding this comment.
ここで filemanager_path と rb_base_dir の既定値を空文字に変更したため、新規インストール時は install/instprocessor.php が既存 config.inc.php / system_settings を参照できないケースで空のまま config.inc.php に書き込まれます。結果として config('filemanager_path') / config('rb_base_dir') が空になり、manager/actions/element/files.dynamic.php や manager/media/browser/... が期待するルートパスが未設定のままになってファイルマネージャ/ファイルブラウザが利用できなくなります（手動で設定を保存するまで復旧しない）。
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR prevents filemanager_path and rb_base_dir from being lost during upgrades by migrating these settings from the database to the config file, with environment variable support as the top priority.
Changes:
- Added logic to preserve existing
filemanager_pathandrb_base_dirvalues from config files during installation/upgrade - Implemented environment variable support (MODX_FILEMANAGER_PATH, MODX_RB_BASE_DIR) with fallback to config file values
- Prevented these path settings from being inappropriately saved to or read from the database
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| install/instprocessor.php | Added functions to read path settings from existing config file and system_settings table, populate config template placeholders, and delete migrated settings from database |
| install/tpl/config.inc.tpl | Updated template to output filemanager_path and rb_base_dir with env() wrapper for environment variable support |
| manager/includes/document.parser.class.inc.php | Added resolvePathConfig() method to prioritize environment variables over global variables for path resolution |
| manager/includes/config_check.inc.php | Removed direct database queries for these settings, now uses evo()->config() with safe empty string handling |
| manager/processors/save_settings.processor.php | Added guards to skip these path settings in the save loop and added safe empty string checks in setPermission() |
| manager/includes/default.config.php | Changed default values from MODX_BASE_PATH-based paths to empty strings since values are now resolved elsewhere |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
yama
changed the title
filemanager_path / rb_base_dir を既存 config から優先取得して破棄を防止## 主な変更内容 **環境依存パス設定を `config.inc.php` に移行** データベースで管理していた `filemanager_path` と `rb_base_dir` を `config.inc.php` での管理に変更し、環境依存設定をコード管理可能にしました。 ### 変更詳細 1. **パス設定の読み込み改善** - `ex_dbapi.php`: `config.inc.php` 読み込み前に `global` 宣言を追加し、パス変数をグローバルスコープに正しく格納 - `DocumentParser::getSettings()`: グローバル変数でDBの設定値を上書きし、`config.inc.php` の値を優先 2. **グローバル設定画面の対応** - `functions.inc.php`: グローバル設定表示時に `config.inc.php` の値を反映 - `save_settings.processor.php`: 設定保存時に `config.inc.php` を更新する `update_config_inc()` 関数を実装 - 変更がない場合はファイル更新をスキップし、不要なパーミッション変更を回避 3. **テンプレート統合** - config.inc.tpl: インストーラのテンプレートをコア領域に移動 - `instprocessor.php`: コア側のテンプレートを参照するように変更 - SSOT原則に従い、テンプレートを1箇所で管理 ### 影響範囲 - 新規インストール: `config.inc.php` でパス設定を管理 - 既存環境: グローバル設定を保存すると `config.inc.php` が自動更新される - 設定値は環境変数 (`MODX_FILEMANAGER_PATH` / `MODX_RB_BASE_DIR`) でのオーバーライドに対応
1 participant
概要