Skip to content

chore(config): refactor session-ping-limit setting to valid ConfigLexicon name pattern with underscores - #19223

Open
sudormant wants to merge 1 commit into
mainfrom
refactor/config/RefactorTalkConfigToAppConfigSessionPingLimit
Open

chore(config): refactor session-ping-limit setting to valid ConfigLexicon name pattern with underscores#19223
sudormant wants to merge 1 commit into
mainfrom
refactor/config/RefactorTalkConfigToAppConfigSessionPingLimit

Conversation

@sudormant

Copy link
Copy Markdown
Contributor

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not possible
  • 📘 API documentation in docs/ has been updated or is not required
  • 🔖 Capability is added or not needed

@sudormant sudormant added this to the ☃️ Next Major (36) milestone Sep 1, 2026
@sudormant sudormant self-assigned this Sep 1, 2026
@sudormant sudormant added feature: settings ⚙️ Settings and config related issues technical debt labels Sep 1, 2026
Comment thread lib/ConfigLexicon.php Outdated
new Entry(Config::CREATE_SAMPLES, ValueType::BOOL, true, definition: 'Create sample conversations (the content can be overwritten by providing files in a provided `samples_directory` app config)'),
new Entry(Config::MATTERBRIDGE_ENABLED, ValueType::BOOL, false, definition: 'Whether the Matterbridge integration is enabled and can be configured'),
new Entry(Config::DELETE_ONE_TO_ONE_CONVERSATIONS, ValueType::BOOL, false, definition: 'Whether one-to-one conversations can be left by either participant or should be deleted when one participant leaves'),
new Entry(Config::SESSION_PING_LIMIT, ValueType::INT, 200, definition: 'Number of sessions the HPB can ping in a single request'),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also rename option here and CI is failing

@nickvergessen

Copy link
Copy Markdown
Member

Needs a rebase

…icon name pattern with underscores

Signed-off-by: Christian Lorang <christian.lorang@nextcloud.com>
@sudormant
sudormant force-pushed the refactor/config/RefactorTalkConfigToAppConfigSessionPingLimit branch from b7a8c5a to 419a454 Compare September 3, 2026 09:07
@sudormant

Copy link
Copy Markdown
Contributor Author

Needs a rebase

rebased, but composer test:unit is still failing:

There were 4 errors:

  1. OCA\Talk\Tests\Unit\CapabilitiesTest::testGetCapabilitiesGuest
    TypeError: MockObject_IAppConfig_06b225c2::getAppValueInt(): Return value must be of type int, null returned

/var/www/html/apps-extra/spreed/lib/Capabilities.php:335
/var/www/html/apps-extra/spreed/tests/php/CapabilitiesTest.php:271

  1. OCA\Talk\Tests\Unit\CapabilitiesTest::testGetCapabilitiesUserAllowed with data set #0 (true, false, 'none', true, 1, '')
    TypeError: MockObject_IAppConfig_06b225c2::getAppValueInt(): Return value must be of type int, null returned

/var/www/html/apps-extra/spreed/lib/Capabilities.php:335
/var/www/html/apps-extra/spreed/tests/php/CapabilitiesTest.php:390

  1. OCA\Talk\Tests\Unit\CapabilitiesTest::testGetCapabilitiesUserAllowed with data set Rename app id to spreed #1 (false, true, '1 MB', true, 0, 'en')
    TypeError: MockObject_IAppConfig_06b225c2::getAppValueInt(): Return value must be of type int, null returned

/var/www/html/apps-extra/spreed/lib/Capabilities.php:335
/var/www/html/apps-extra/spreed/tests/php/CapabilitiesTest.php:390

  1. OCA\Talk\Tests\Unit\CapabilitiesTest::testGetCapabilitiesUserAllowed with data set Add input fields for configuration page #2 (false, true, '0 B', false, 0, 'de')
    TypeError: MockObject_IAppConfig_06b225c2::getAppValueInt(): Return value must be of type int, null returned

/var/www/html/apps-extra/spreed/lib/Capabilities.php:335
/var/www/html/apps-extra/spreed/tests/php/CapabilitiesTest.php:390

appConfig doesn't return default value or 0, even setting $this->appConfig->setAppValueInt(Config::SESSION_PING_LIMIT, 200); right before the test doesn't change the test result.

Happens with some refactored settings to appConfig, but not others. haven't found the root issue quite yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: settings ⚙️ Settings and config related issues technical debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants