Skip to content

Add switch to manage the smarthost between cluster and Nextcloud#175

Open
mrmarkuz wants to merge 4 commits intomainfrom
smarthostswitch
Open

Add switch to manage the smarthost between cluster and Nextcloud#175
mrmarkuz wants to merge 4 commits intomainfrom
smarthostswitch

Conversation

@mrmarkuz
Copy link
Member

  • Add UI switch
  • Add required env variables
  • Remove smarthost settings when smarthost is disabled
  • Add update script to allow Nextcloud or cluster smarthost management

NethServer/dev#7871

Copy link

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 adds a UI toggle to allow administrators to choose whether email sending for Nextcloud should be managed by the cluster's smarthost or configured directly within Nextcloud's admin panel. This provides flexibility for installations that need custom email configurations in Nextcloud.

Changes:

  • Added a toggle switch in the Settings UI to control the internal_smarthost setting
  • Added environment variable INTERNAL_SMARTHOST to track whether Nextcloud manages its own email settings
  • Modified discover-smarthost and setup-smtp to respect the internal smarthost flag
  • Added migration script to detect and preserve existing email configuration during updates

Reviewed changes

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

Show a summary per file
File Description
ui/src/views/Settings.vue Added NsToggle component for internal_smarthost setting with data binding to backend
ui/public/i18n/en/translation.json Added English translation label for the internal smarthost toggle
imageroot/update-module.d/17smarthost_switch Migration script to detect whether cluster or Nextcloud was managing email in existing installations
imageroot/bin/setup-smtp Modified to only apply cluster smarthost settings when INTERNAL_SMARTHOST is False, and to clean up Nextcloud email config when disabled
imageroot/bin/discover-smarthost Modified to skip writing smarthost.env when internal_smarthost is enabled
imageroot/actions/get-configuration/20read Added default value for internal_smarthost field
imageroot/actions/configure-module/20configure Added INTERNAL_SMARTHOST environment variable with default value
Comments suppressed due to low confidence (5)

imageroot/update-module.d/17smarthost_switch:25

  • The occget function is called without checking if the Nextcloud container is running. If Nextcloud is not running during the update, this subprocess call will fail and the update script will crash. Consider adding error handling or checking container status before calling occget.
smtphost = occget(["config:system:get","mail_smtphost"])

ui/public/i18n/en/translation.json:50

  • The label "Manage email sending from Nextcloud admin panel" is unclear about what enabling/disabling this toggle does. When enabled, does Nextcloud manage email (internal smarthost), or does the cluster manage it? Consider a clearer label such as "Use Nextcloud email settings" or "Configure email from Nextcloud admin panel" to make it clear that enabling this means Nextcloud manages its own email settings.
        "internal_smarthost": "Manage email sending from Nextcloud admin panel"

imageroot/update-module.d/17smarthost_switch:29

  • Missing space after equals sign. Should be 'smtpsmarthost = smarthost['SMTP_HOST']' for consistency with Python PEP 8 style guidelines and the rest of the codebase.
    smtpsmarthost=smarthost['SMTP_HOST']

imageroot/update-module.d/17smarthost_switch:36

  • The migration logic has an edge case: if both mail_smtphost and SMTP_HOST are empty/unset, they will match and INTERNAL_SMARTHOST will be set to False (cluster management). However, in this case, there's no cluster smarthost configured, so it's unclear which mode should be selected. Consider adding a check to handle the case where both values are empty, and default to INTERNAL_SMARTHOST=True in that case, since the cluster isn't actually managing email.
if smtphost == smtpsmarthost:
    config['INTERNAL_SMARTHOST']=False
    agent.write_envfile('config.env', config)

imageroot/update-module.d/17smarthost_switch:31

  • Missing space after equals sign. Should be 'smtpsmarthost = ""' for consistency with Python PEP 8 style guidelines and the rest of the codebase.
    smtpsmarthost=""

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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