fix overriding configuration

[rommelfreddy]

This fixes an issue that if you save the configuration within the administration, the sentry error reporting got disabled, because of the setting sentry/environment/enabled

this PR will add another setting with which it is possible to enable to override.
Only if the override is enabled, it is possible to disable the Sentry error tracking.

Also reworked the collectModuleConfig within the config-helper to make sure that values, which are null or empty, do not override the deployment configuration.

I added canRestore to the system.xml to make sure that no empty values got stored into the database.

I removed the TableNotFoundException-Handling, because i do not see a case when this could happen.

[indykoning]

Awesome i can't wait to get this merged!

The TableNotFoundException had been introduced in #125
I could personally reproduce this in the few cases of setting up the project with sentry already installed, or in deployment pipelines where the code would be built but no database connection could be available.

Could you try the steps in that PR and see if it is no longer an issue with your current code? 🙂

[rommelfreddy]

Awesome i can't wait to get this merged!

The TableNotFoundException had been introduced in #125 I could personally reproduce this in the few cases of setting up the project with sentry already installed, or in deployment pipelines where the code would be built but no database connection could be available.

Could you try the steps in that PR and see if it is no longer an issue with your current code? 🙂

I do have deployments with gitlab and without a database.
In addition the config.php with the sentry configuration is also checked in.

I do not have any issues/problems with this exception

[rommelfreddy]

@indykoning rebased the changes

[indykoning]

Seems like this has been fixed by the addition of canRestore="1" in #155 released in 3.9.0

[rommelfreddy]

i do not think the PR #155 fix this issue from this PR.
have you explicit tested it?

I think it would in better in general, if there is only one configuration -> system-config.
It is also possible to have a deployment-configuration within the system-config by placing the config in env.php.
So we could get rid of the deployment-configuration.
I do not see any reason why we should have a deployment-configuration.

and with the removal of this, we do not have issues with overriding the deployment-configuration within the system-config.
Should I create a PR for this?

[rommelfreddy]

@indykoning

[indykoning]

Yes I have indeed checked it, the reason that it would fix it for installations is that by default "use default" is checked, even on the global scope.
So when you save it, it will not have the values to the database.

The only exception would be if it's already in the database for which it would make sense that it's not checked.

I'm not sure everything maps the same way as expected between deployment config and system config.
We could start moving away from deployment config but it would certainly be a major release in need of migration steps