Allow disable telemetry from BrandingConstants #115
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jonbartels
requested changes
Jun 2, 2025
mgaffigan
force-pushed
the
feature/branding-connectservice
branch
from
a048024 to
346e1a4
Compare
Contributor
Author
|
Ok. I think all comments have been addressed. Let me know if I missed something. |
jonbartels
previously approved these changes
Jun 5, 2025
Contributor
jonbartels
left a comment
jonbartels
left a comment
There was a problem hiding this comment.
LGTM Thank you for this PR!
It solves some bigger issues and questions using very little code.
jonbartels
requested review from
a team,
gibson9583,
kayyagari,
kpalang,
pacmano1,
ssrowe and
tonygermano
and removed request for
a team
kayyagari
reviewed
Jun 5, 2025
kayyagari
previously approved these changes
Jun 5, 2025
tonygermano
requested changes
Jun 5, 2025
server/src/com/mirth/connect/client/core/ConnectServiceUtil.java
Outdated
Show resolved
Hide resolved
mgaffigan
dismissed stale reviews from kayyagari, gibson9583, and jonbartels
via
cb867db
Member
|
@mgaffigan I pushed my changes. If all look good to you I'm going to rebase and squash this PR. I moved all of the constants to the client-core class, because I didn't like having them split up and some duplicated. As a result I used static imports in several places. I also put a few more safety checks in place and show/hide additional gui components based on the settings. See screenshots below: CENTRAL_USER_REGISTRATION=true; MANDATORY_USER_REGISTRATION=false; registration box not checked
CENTRAL_USER_REGISTRATION=true; MANDATORY_USER_REGISTRATION=false; registration box checked
CENTRAL_USER_REGISTRATION=true; MANDATORY_USER_REGISTRATION=true
CENTRAL_USER_REGISTRATION=false
CHECK_FOR_NOTIFICATIONS=false
CHECK_FOR_NOTIFICATIONS=true
SEND_USAGE_STATISTICS=false
SEND_USAGE_STATISTICS=true
|
tonygermano
approved these changes
Jun 6, 2025
jonbartels
requested review from
a team,
gibson9583,
jonbartels and
kayyagari
and removed request for
a team
jonbartels
approved these changes
Jun 6, 2025
kayyagari
approved these changes
Jun 6, 2025
Contributor
Author
|
@tonygermano, I like the disabled UI for mandatory. Much more clear. Am I missing something re: stats? I was unaware of the setting, but don't see anything that checks it in Mirth.java:404 nor ConnectServiceUtil.java:227. I only see it checked in front-end. It should not be an issue given |
Adds the following settings to BrandingConstants in client-core: - CENTRAL_USER_REGISTRATION - When false, disables the sending of user demographic information and hides the registration and marketing consent boxes from the first time login screen. - MANDATORY_USER_REGISTRATION - used in conjunction with CENTRAL_USER_REGISTRATION. When this is setting is true and the user id is 1, then the user will not be able to unselect the registration checkbox. - CHECK_FOR_NOTIFICATIONS - When false, allows for notification fetching from the client to be disabled. The notification menu task is also removed. - SEND_USAGE_STATISTICS - When false, allows for sending of usage statistics to be disabled. Also hides the setting which allows users to opt-in or opt-out of sending statistics. With the restoration of the registration checkbox when the appropriate settings are enabled, the adjustment of which fields are required on the user profile has also been restored. Co-authored-by: Tony Germano <tony@germano.name> Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net> Signed-off-by: Tony Germano <tony@germano.name> Issue: OpenIntegrationEngine#5
tonygermano
force-pushed
the
feature/branding-connectservice
branch
from
e3ffc1e to
3954bc5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #5 by adding branding constants:
As is, stats and registrations are sent to the non-existent https://connect.openintegrationengine.org/. Not sure what is desired, but I presume it is easier to keep it enabled than to try to merge a re-enable later.
This also updates the HttpClient configuration for notifications and telemetry to use the system defaults. I presume the preceding was an over-zealous edit during the "no PHI over anything but TLS1.2" heyday.
Related: #114
Alternative: #107