Moved cc log admin to /apps settings

[levb]

Summary

• Moved the log settings under /apps settings
• Also restored simple JSON marshaling of StoredConfig by using pointers

Ticket Link

none

Screenshots

[javaguirre]

I will test the following cases:

• [x] Activate logs creating a new channel
• [x] Activate logs selecting an existing channel
• [x] Receive logs on a channel without the --json flag.
• [x] Receive logs on a channel with the --json flag.
• [x] Deactivating developer mode and/or testing commands with stop logging
• [x] If we create a channel and it was created, it's not created twice
• [ ] Setting a different --level will filter which logs to receive.
• [ ] If we specify both --create-channel and --channel, it fails.
• [ ] Logs should appear in a channel from the team where I executed the command.

[javaguirre]

Not sure if this is a bug or not, but if I have a channel selected, I open the settings and try to select 'create a new channel', the option is not there, I have to persist the settings with the 'do not copy app logs to a channel', then open the settings again to select the 'create channel' selection (see video).

https://user-images.githubusercontent.com/488556/208077069-43e5cb28-fdac-4c99-b0ff-c0f43fd1b18e.mov

[javaguirre]

If I disable the 'Developer Mode' in MM System Console, the apps settings won't check it's disabled and it will tell it's enabled, I think both should be in the same stage since we're talking about the same? Is this a different Developer Mode?

https://user-images.githubusercontent.com/488556/208078557-8977f21a-6309-44aa-883d-64161eebdee6.mov

[levb]

@javaguirre the /apps settings command provides for overriding the system-wide "Dev mode" (dev+test), so even when it's off an admin can turn it on, for the apps plugin only. Ditto HTTP. So, when the override mode is on, the overrides are used; when it's off - the system-wide settings are.

If the override mode in the apps plugin is off, the plugin should receive OnConfigurationChanged when you apply changes in the system console, and adjust accordingly. If it's not happening - it's a bug.

[javaguirre]

@javaguirre the /apps settings command provides for overriding the system-wide "Dev mode" (dev+test), so even when it's off an admin can turn it on, for the apps plugin only. Ditto HTTP. So, when the override mode is on, the overrides are used; when it's off - the system-wide settings are.

If the override mode in the apps plugin is off, the plugin should receive OnConfigurationChanged when you apply changes in the system console, and adjust accordingly. If it's not happening - it's a bug.

Thank you for the clarification! I understand now. 👍

[javaguirre]

When I try to set the --level of the apps logs debug command, the autocomplete works, but when I submit the action it fails (attachment).

[levb]

@javaguirre that's weird, the debug logs command should be gone entirely... refresh your client? (bindings?)

[javaguirre]

@javaguirre that's weird, the debug logs command should be gone entirely... refresh your client? (bindings?)

I thought so, I executed a make restart in the mattermost-server so it builds the frontend too, the webapp notified me to refresh, but the bindings are still there, I might be doing something wrong, not sure if I have to clean something else, I thought it was enough with the restart command.

[javaguirre]

When I select a level and open the settings again, the current level is not selected, I think that would be nice.

https://user-images.githubusercontent.com/488556/208082153-5d20d0c1-c905-4aae-91ee-6869ed2ee39e.mov

[javaguirre]

This is no longer happening 👏

https://github.com/mattermost/mattermost-plugin-apps/pull/394#issuecomment-1347948166

[levb]

the current level is not selected

@javaguirre ditto JSON, thx! Forgot...

[codecov-commenter]

Codecov Report

Base: 20.89% // Head: 20.84% // Decreases project coverage by -0.05% :warning:

Coverage data is based on head (ac2df7f) compared to base (44db542). Patch coverage: 2.00% of modified lines in pull request are covered.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #414 +/- ## ========================================== - Coverage 20.89% 20.84% -0.06% ========================================== Files 80 80 Lines 5270 5283 +13 ========================================== Hits 1101 1101 - Misses 4040 4053 +13 Partials 129 129 ``` | [Impacted Files](https://codecov.io/gh/mattermost/mattermost-plugin-apps/pull/414?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost) | Coverage Δ | | |---|---|---| | [server/config/config.go](https://codecov.io/gh/mattermost/mattermost-plugin-apps/pull/414?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-c2VydmVyL2NvbmZpZy9jb25maWcuZ28=) | `37.50% <ø> (+9.37%)` | :arrow_up: | | [server/config/service.go](https://codecov.io/gh/mattermost/mattermost-plugin-apps/pull/414?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-c2VydmVyL2NvbmZpZy9zZXJ2aWNlLmdv) | `0.00% <0.00%> (ø)` | | | [server/config/test\_service.go](https://codecov.io/gh/mattermost/mattermost-plugin-apps/pull/414?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-c2VydmVyL2NvbmZpZy90ZXN0X3NlcnZpY2UuZ28=) | `0.00% <0.00%> (ø)` | | | [server/plugin.go](https://codecov.io/gh/mattermost/mattermost-plugin-apps/pull/414?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-c2VydmVyL3BsdWdpbi5nbw==) | `42.15% <12.50%> (-1.73%)` | :arrow_down: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[levb]

@javaguirre I added the current value selection for Level and JSON

[javaguirre]

@javaguirre I added the current value selection for Level and JSON

I tested it and It works great 👍

[javaguirre]

https://user-images.githubusercontent.com/488556/210335874-9064006e-d160-4d0c-9129-6bedb9470108.mov

When I enable the developer mode I can't select the channel for logging until I persist that change. I have to open the modal again and then it's available to select.

I was expecting to change both at the same time, is it possible?

[javaguirre]

@javaguirre the /apps settings command provides for overriding the system-wide "Dev mode" (dev+test), so even when it's off an admin can turn it on, for the apps plugin only. Ditto HTTP. So, when the override mode is on, the overrides are used; when it's off - the system-wide settings are.

If the override mode in the apps plugin is off, the plugin should receive OnConfigurationChanged when you apply changes in the system console, and adjust accordingly. If it's not happening - it's a bug.

Did you manage to fix this one?

[levb]

For the video - not currently fixable, filed https://mattermost.atlassian.net/browse/MM-49451

As for setting the dev mode - the apps plugin will not set the server's dev mode, only it's own. Sorry for the confusion.

Consider both resolved?

[levb]

@hanzei @mickmister Can I please get a 2nd review, I'd like to merge it soon now that 1.2 is out

[DHaussermann]

Tested and passed All config are functional. It is a bit confusing that some fields must be set first. I was also a bit confused that you can't simply change the channel the logs get written to. Logging to a channel must be disable and re-enabled.

@levb seeing as how the snags are minor and this is a debug tool - We can see if UX improvements are worth the effort later.

LGTM!

[hanzei]

@levb Heads up that this PR will need to update the docs that were originally added in https://github.com/mattermost/mattermost-developer-documentation/pull/1181