Skip to content

Store settings in config/ and add import/export#49

Merged
dvir001 merged 7 commits intomainfrom
2026-03-10-ReworkConfig
Mar 11, 2026
Merged

Store settings in config/ and add import/export#49
dvir001 merged 7 commits intomainfrom
2026-03-10-ReworkConfig

Conversation

@dvir001
Copy link
Copy Markdown
Owner

@dvir001 dvir001 commented Mar 10, 2026

Move application settings into a dedicated config/ directory and unify email settings into the single app_settings.json file. Update simple_org_chart.config to expose CONFIG_DIR/REPO_DIR and ensure directories are created. Modify email_config and settings save/load logic to read/write the unified SETTINGS_FILE while preserving unrelated keys (so email/settings don't overwrite each other). Add API endpoints (/api/settings/export and /api/settings/import) and corresponding UI (configure page, JS, and locale entries) to export/import configuration. Change user-scanner behavior to use GitHub releases (with PyPI fallback) and add UI-driven manual install flow instead of auto-install. Update Dockerfiles and compose files to mount ./config, add gosu and an entrypoint script to fix bind-mount permissions before dropping privileges. Update tests to expect the new config location and adjust other frontend and backend code to handle missing DOM elements and defaults. Overall this enables persistent, portable settings and safer container bind-mount handling.

Move application settings into a dedicated config/ directory and unify email settings into the single app_settings.json file. Update simple_org_chart.config to expose CONFIG_DIR/REPO_DIR and ensure directories are created. Modify email_config and settings save/load logic to read/write the unified SETTINGS_FILE while preserving unrelated keys (so email/settings don't overwrite each other). Add API endpoints (/api/settings/export and /api/settings/import) and corresponding UI (configure page, JS, and locale entries) to export/import configuration. Change user-scanner behavior to use GitHub releases (with PyPI fallback) and add UI-driven manual install flow instead of auto-install. Update Dockerfiles and compose files to mount ./config, add gosu and an entrypoint script to fix bind-mount permissions before dropping privileges. Update tests to expect the new config location and adjust other frontend and backend code to handle missing DOM elements and defaults. Overall this enables persistent, portable settings and safer container bind-mount handling.
Copilot AI review requested due to automatic review settings March 10, 2026 17:36
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR relocates persisted configuration into a dedicated config/ directory, consolidates email + app settings into a single app_settings.json, and adds UI/API support for importing/exporting that configuration. It also adjusts container deployment to use bind mounts and a permission-fixing entrypoint, and updates the user-scanner workflow to require explicit installation and to check GitHub releases for updates.

Changes:

  • Move settings persistence to config/app_settings.json, ensure required directories exist, and preserve unrelated keys when saving settings/email config.
  • Add config export/import UI and API endpoints; adjust reports/configure pages for user-scanner “manual install” behavior.
  • Update Docker packaging/compose to bind-mount ./config and run an entrypoint that fixes permissions before dropping privileges.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
simple_org_chart/config.py Adds CONFIG_DIR/REPO_DIR, moves SETTINGS_FILE to config/, expands ensure_directories().
simple_org_chart/settings.py Updates settings save path and switches to read/merge/write behavior to preserve unrelated keys.
simple_org_chart/email_config.py Migrates email config into shared app_settings.json and preserves non-email keys.
simple_org_chart/app_main.py Adds /api/settings/export + /api/settings/import; resets both app/email defaults on reset.
static/configure.js Adds export/import handlers and user-scanner install UI flow.
templates/configure.html Adds Configuration Transfer section and user-scanner install row.
static/locales/en-US.json Adds locale strings for config transfer and user-scanner install UI.
static/reports.js Stops auto-install of user-scanner and hides report option unless enabled and installed.
simple_org_chart/user_scanner_service.py Checks GitHub releases for latest version (with PyPI fallback).
Dockerfile Installs gosu, adds entrypoint, creates config/ and changes permissions.
deploy/entrypoint.sh New entrypoint to chown bind-mounted dirs then drop privileges.
docker-compose.yml Switches from named volumes to bind mounts including ./config.
docker-compose-dev.yml Same bind-mount change for dev compose.
tests/test_config.py Updates expectations for settings file location and directory creation.
tests/test_email_schedule.py Updates email schedule tests to patch SETTINGS_FILE in shared settings file.

@dvir001
Copy link
Copy Markdown
Owner Author

dvir001 commented Mar 10, 2026

@copilot open a new pull request to apply changes based on the comments in this thread

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 10, 2026

@dvir001 I've opened a new pull request, #50, to work on those changes. Once the pull request is ready, I'll request review from you.

…d i18n in install flow (#50)

* Initial plan

* Address all PR review comments: locking, i18n, security, naming, and permissions

Co-authored-by: dvir001 <39403717+dvir001@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: dvir001 <39403717+dvir001@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.

@dvir001
Copy link
Copy Markdown
Owner Author

dvir001 commented Mar 10, 2026

@copilot open a new pull request to apply changes based on the comments in this thread

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 10, 2026

@dvir001 I've opened a new pull request, #51, to work on those changes. Once the pull request is ready, I'll request review from you.

…rd (#51)

* Initial plan

* Fix inter-process locking, email key restriction, dict check, and conditional settings save

Co-authored-by: dvir001 <39403717+dvir001@users.noreply.github.com>

* Remove committed lock file and add config/*.lock to .gitignore

Co-authored-by: dvir001 <39403717+dvir001@users.noreply.github.com>

* Fix fd leak in lock acquire, remove locked() race, and extract _filter_email_keys helper

Co-authored-by: dvir001 <39403717+dvir001@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: dvir001 <39403717+dvir001@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 4 comments.

@dvir001
Copy link
Copy Markdown
Owner Author

dvir001 commented Mar 11, 2026

@copilot open a new pull request to apply changes based on the comments in this thread

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 11, 2026

@dvir001 I've opened a new pull request, #52, to work on those changes. Once the pull request is ready, I'll request review from you.

* Initial plan

* Fix lock path at acquire-time, i18n hardcoded strings, entrypoint symlink safety

Co-authored-by: dvir001 <39403717+dvir001@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: dvir001 <39403717+dvir001@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

static/configure.js:2437

  • _initUserScannerConfigUI() is called every time applySettings() runs (e.g., after save/import), but checkBtn.addEventListener('click', ...) is registered on each call. This will accumulate duplicate listeners and can trigger multiple concurrent update-check requests per click. Prefer using checkBtn.onclick = ..., or register listeners once (e.g., guard with a flag / data-* attribute) and only update UI state on subsequent calls.
    if (checkBtn) {
        checkBtn.addEventListener('click', async () => {
            checkBtn.disabled = true;
            if (updateStatusEl) updateStatusEl.textContent = resolveTranslation('configure.userScanner.update.checking', 'Checking…');
            if (applyBtn) applyBtn.style.display = 'none';
            try {
                const resp = await fetch(`${window.location.origin}/api/user-scanner/check-update`, { credentials: 'include' });
                const info = await resp.json();
                if (info.updateAvailable) {
                    if (updateStatusEl) updateStatusEl.textContent = resolveTranslation('configure.userScanner.update.availablePrefix', 'Update available: v') + info.currentVersion + resolveTranslation('configure.userScanner.update.availableArrow', ' → v') + info.latestVersion;
                    if (applyBtn) applyBtn.style.display = '';
                } else {
                    const ver = info.currentVersion || info.latestVersion || resolveTranslation('configure.userScanner.update.unknownVersion', 'unknown');
                    if (updateStatusEl) updateStatusEl.textContent = resolveTranslation('configure.userScanner.update.upToDatePrefix', 'Up to date (v') + ver + resolveTranslation('configure.userScanner.update.upToDateSuffix', ')');
                }
            } catch (err) {
                if (updateStatusEl) updateStatusEl.textContent = resolveTranslation('configure.userScanner.update.checkFailed', 'Failed to check for updates.');
            } finally {
                checkBtn.disabled = false;
            }
        });

static/configure.js:2467

  • Similar to the update-check button, applyBtn.addEventListener('click', ...) is added each time _initUserScannerConfigUI() runs, which can accumulate duplicate listeners and submit multiple update requests on a single click after settings reloads. Consider binding this handler once (or use onclick) and keep _initUserScannerConfigUI() focused on rendering state.
    if (applyBtn) {
        applyBtn.addEventListener('click', async () => {
            applyBtn.disabled = true;
            if (updateStatusEl) updateStatusEl.textContent = resolveTranslation('configure.userScanner.update.updating', 'Updating…');
            try {
                const resp = await fetch(`${window.location.origin}/api/user-scanner/update`, {
                    method: 'POST',
                    credentials: 'include',
                });
                const result = await resp.json();
                if (result.success) {
                    if (updateStatusEl) updateStatusEl.textContent = resolveTranslation('configure.userScanner.update.updatedPrefix', 'Updated to v') + result.version;
                    if (statusEl) {
                        const verSpan = statusEl.querySelector('span');
                        if (verSpan) verSpan.textContent = resolveTranslation('configure.userScanner.status.installedPrefix', 'Installed — v') + result.version;
                    }
                    applyBtn.style.display = 'none';
                } else {
                    if (updateStatusEl) updateStatusEl.textContent = result.error || resolveTranslation('configure.userScanner.update.failed', 'Update failed.');
                }
            } catch (err) {
                if (updateStatusEl) updateStatusEl.textContent = resolveTranslation('configure.userScanner.update.failedPrefix', 'Update failed: ') + err.message;
            } finally {
                applyBtn.disabled = false;
            }
        });
    }

dvir001 and others added 3 commits March 11, 2026 15:42
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@dvir001 dvir001 merged commit e00ced2 into main Mar 11, 2026
7 checks passed
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.

3 participants