Skip to content
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

improve config panel #356

Merged
merged 98 commits into from
Sep 13, 2023
Merged

improve config panel #356

merged 98 commits into from
Sep 13, 2023

Conversation

Gredin67
Copy link

@Gredin67 Gredin67 commented Jan 6, 2023

  • Expose more parameters
  • Use bind __FOOBAR__
  • Link to first element_ynh instance element

TODO :

@Gredin67
Copy link
Author

Gredin67 commented Jan 6, 2023

!testme

@yunohost-bot
Copy link

Alrighty!
Test Badge

@Gredin67
Copy link
Author

Gredin67 commented Jan 9, 2023

!testme

1 similar comment
@Gredin67
Copy link
Author

Gredin67 commented Jan 9, 2023

!testme

@yunohost-bot
Copy link

🎠
Test Badge

@Gredin67
Copy link
Author

@Josue-T it's starting to work, but I don't know what to do with
e2e_enabled_by_default, which is not a boolean in homeserver.yaml, but

        [client.experience.e2e_enabled_by_default]
        ask = "End-to-End Encryption by default for locally-created Rooms"
        type = "select"
        choices = ["all", "invite", "off"]
        bind = "encryption_enabled_by_default_for_room_type:/etc/matrix-__APP__/homeserver.yaml"

You use e2e_enabled_by_default in many places that I don't necessarily understand. Can you sort this out?

@Gredin67
Copy link
Author

correct parameter handling should be double-checked for (non-exhaustive)

    allow_registration
    turn_allow_guests
    sso_enabled
    password_enabled

@Gredin67
Copy link
Author

@thardev can you check /scripts/config to see if it fits latest config_panel standards?
And also check/add sed stuff for allowed_local_3pids and auto_join_rooms

@yunohost-bot
Copy link

May the CI gods be with you!
Test Badge

@Gredin67 Gredin67 mentioned this pull request Aug 27, 2023
2 tasks
@Thatoo
Copy link

Thatoo commented Aug 27, 2023

Tonight on brand new Yunohost and Synapse install :
Install OK
In the config-panel, when I wanted to change "End-to-End Encryption by default for locally-created Rooms" from "off" to "invite", and I wanted to save, I got the following error : "Le formulaire contient des erreurs" , and it didn't change the value in the homserver.yaml

Then, I wanted to change "Largest allowed media upload size in bytes." from 10M to 100M and it failed, here is the yunopaste : https://paste.yunohost.org/raw/avaxisujeg
but it has indeed changed the value in the homserver.yaml

Finally, I tried to change "Server statistics" from off to on and this worked and it has indeed changed the value in the homserver.yaml

@Thatoo
Copy link

Thatoo commented Aug 27, 2023

Enable Registration for new users : worked to change it from false to true but not from true to false... "Le formulaire contient des erreurs"

Disable asking Phone Number in Registration flow worked to change it from true to false but not the opposite : "Le formulaire contient des erreurs"

Auto-Create room for Auto Join if not existing? didn't work at all : "Le formulaire contient des erreurs"

Enable email notifications for new users? didn't work at all : "Le formulaire contient des erreurs"

Access Public Rooms Directory over Federation? didn't work at all : "Le formulaire contient des erreurs"

Disable content sharing inside push notification. didn't work at all : "Le formulaire contient des erreurs"

Allow non-server-admin Users to create Spaces? didn't work at all : "Le formulaire contient des erreurs"

Enable sending emails for messages the user missed? didn't work at all : "Le formulaire contient des erreurs"

@Josue-T Josue-T mentioned this pull request Aug 27, 2023
@Josue-T
Copy link

Josue-T commented Aug 27, 2023

lgtm! @Josue-T can you please merge in testing ? @YunoHost-Apps/matrix-bridges anyone could test upgrade and functionality of the new config panel ?

Maybe we need to investigate the last issues message before to merge it.

@MayeulC
Copy link

MayeulC commented Aug 28, 2023

There is already a mechanism that backs up the old config and warns the user with the diff. If you want to improve this, it should be done in yunohost core, not in this package.

Of course, not necessary for this PR, but as a datapoint, here is what I modify in my config:

  • enable metrics, and add a localhost listener (that I use with yunohost's prometheus package, to then plot everything in grafana)
  • adjust cache factors (both global_factor and sync_response_cache_duration as I have plenty of RAM)
  • whitelist 127.0.0.1 in ip_range_whitelist, as I run a push server (ntfy for UnifiedPush) at this address.

The above could easily be added as options in the config panel (possibly in an "advanced" section).

@Gredin67
Copy link
Author

Gredin67 commented Aug 28, 2023 via email

@rosbeef
Copy link

rosbeef commented Aug 28, 2023

I'm focusing on my last addition account_threepid_delegates_msisdn and enable_registration for now.

I have some clues :

  1. I don't understand why it is at the top of the setting.yml file (maybe sed config script is not working on the first line,(sort of speculation from my part but i read something somewere about that ))
  2. When i run yunohost app config set synapse__2 main.welcome.account_threepid_delegates_msisdn -v 'https://127.0.0.1:2345' do nothing as it don't find any modifications, which is false.
  3. Run yunohost app config get synapse__2 returns me "account_threepid_delegates_msisdn" values as empty, not "none" as others empty values.

Please someone can test if i'm right.
When i put account_threepid_delegates_msisdn at the middle of the setting.yml , setting enable_registration from true to false works.

@rosbeef
Copy link

rosbeef commented Sep 2, 2023

Fix "false" value to "none" in select field
It seems the bug was related to the value of the requires_registration_3pid . The requires_registration_3pid default value setted to false in the install script and was interpreted as a boolean.
So getting the value to show in the config panel generate a bug as 0 is not a valid value to choose in the select type field.
To mitigate i change the default value to none in Install, upgrade scripts and config-panel.toml

please proceed with tests
i will sleep ;)

@gautz gautz mentioned this pull request Sep 12, 2023
2 tasks
@Josue-T
Copy link

Josue-T commented Sep 12, 2023

Hello,

@rosbeef @Gredin67 did you test the last version ? Does everything works ?

@Gredin67
Copy link
Author

Gredin67 commented Sep 12, 2023 via email

@Josue-T Josue-T merged commit 381611b into YunoHost-Apps:testing Sep 13, 2023
@Thatoo
Copy link

Thatoo commented Sep 19, 2023

  • Password login option is not working : it does not change the option in the homserver.yaml file (line 2171)
  • Enable registration option works but it automatically set
registrations_require_3pid:
 - email
 # - msisdn

even though Registration requires all following 3PID personal identifier is set to none.

  • Auto Join new Users in following Rooms option works
  • Auto-Create room for Auto Join if not existing? option works
  • Enable email notifications for new users? option works
  • End-to-End Encryption by default for locally-created Rooms option works
  • Access Public Rooms Directory over Federation? option works
  • Disable content sharing inside push notification option works
  • Element instance your HomeServer should redirect to option works
  • Allow non-server-admin Users to create Spaces? option works
  • Enable sending emails for messages the user missed? option works but I don't understand why it is far down from the Enable email notifications for new users? option , I would imagine notififcation options being close to each other in the list
  • URL for client links within the email notifications. option works
  • Largest allowed media upload size in bytes. option fails, I can't change it from 10M to 100M, I get an error : https://paste.yunohost.org/raw/xahexixizi
  • Backup before upgrade? option was not tested, I don't know where to check that in homeserver.yaml file
  • Server statistics option works
  • Web client location to direct users to during an invite. option works
  • Allow Users to Register as Guests? option works
  • Enable Auto Join Room for Guests? option works
  • Allow discovering friends with phone number or email? option works
  • Identity server suggested to clients? option works
  • Access Public Rooms Directory without authentification? option works
  • Shared Secret for Registration. option works
  • Should guests be allowed to use the TURN server? option works

@rosbeef
Copy link

rosbeef commented Sep 20, 2023

Sorry i have no time till monday. But this error Come from the $domain variable which does not exist.

@rosbeef
Copy link

rosbeef commented Sep 20, 2023

  • Enable registration option works but it automatically set
registrations_require_3pid:
 - email
 # - msisdn

For this maybe those values are the default in config file and should be set to all commented.

I vote for removing the enable registration switch in the install panel too. This should simplify install for non techies and simplify scripts

@Thatoo
Copy link

Thatoo commented Sep 20, 2023

I just realize that it is quiet urgent as testing has been merged in master now...

@rosbeef
Copy link

rosbeef commented Sep 21, 2023

Dmne is it possible to suspend upgrade?

@rosbeef
Copy link

rosbeef commented Sep 21, 2023

Sorry, just to be informed, how is the "workflow" to merge testing into master generally in yunohost apps??
I will try to do something Saturday, but till this I'm only with my mobile and it's not really usable.

@Gredin67
Copy link
Author

Gredin67 commented Sep 22, 2023 via email

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.

8 participants