Skip to content

Conversation

@AndyButland
Copy link
Contributor

@AndyButland AndyButland commented Oct 31, 2025

Prerequisites

  • I have added steps to test this contribution in the description below

Please also read the internal chat in the CMS group Slack channel.

Description

Investigating the newsletter sign-up feature on the installation screen I found a few issues:

  • The server-side model was hardcoded to false.
  • The serialization to a request that was expected to be FormUrlEncoded was incorrectly JSON
  • A service on a legacy internal system was being used to collect email addresses.

This PR fixes this by:

  • Accepting the value of the checkbox from the installer setup form.
  • Updating to use the same service as runs to collect sign-ups from the default tour in Umbraco 13.

One little addition - I removed the logging of the user name (possibly email address) when recording the telemetry selection, as we shouldn't really have this in the logs.

Also fixed indentation, so best to review with "hide whitespace".

Testing

Install Umbraco and tick the box to sign-up with an email address that won't already be signed up for Umbraco newsletters.
You should get an email from "Lars from Umbraco" indicating that the sign-up has completed.
The success or otherwise is also written to the log file.

Copilot AI review requested due to automatic review settings October 31, 2025 16:10
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the installer's user creation step to improve newsletter subscription handling and logging. The changes modernize the newsletter subscription process by updating the endpoint and improving error handling with proper logging.

Key changes:

  • Adds logger injection to CreateUserStep for better observability of newsletter subscription operations
  • Updates newsletter subscription to use new endpoint (emailcollector.umbraco.io/api/EmailProxy) with proper data model
  • Replaces silent failure handling with structured logging for subscription attempts
  • Removes username logging from telemetry consent for privacy reasons

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File Description
src/Umbraco.Infrastructure/Installer/Steps/CreateUserStep.cs Added logger parameter, new obsolete constructor for backward compatibility, updated newsletter subscription logic with new endpoint and EmailModel, improved error handling with logging, removed unused using statements
src/Umbraco.Core/Services/MetricsConsentService.cs Removed username from telemetry level logging and formatted code for readability
src/Umbraco.Cms.Api.Management/ViewModels/Installer/UserInstallRequestModel.cs Changed SubscribeToNewsletter property from get-only to get/set to allow proper data binding

Copy link
Contributor

@Migaroez Migaroez left a comment

Choose a reason for hiding this comment

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

Looks good and works as described.

@Migaroez Migaroez merged commit ca08652 into main Nov 7, 2025
26 checks passed
@Migaroez Migaroez deleted the v17/bugfix/subscribe-to-newsletter branch November 7, 2025 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants