chore: [SDK-4220] add Live Activities support to demo app#170
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR includes a public API change (deprecating ILiveActivities.Exit), deletion of 5 GitHub Actions workflows, and a pre-built binary bundle — these warrant a human look beyond the demo-app scope.
Extended reasoning...
Overview
This PR adds Live Activities support to the DotNet MAUI demo app across 29 files. Most changes are demo-app-only (new UI section, DotEnv helper, REST API service, widget extension, button styles), but a meaningful subset touches the SDK public interface and repository infrastructure.
Security risks
No significant security risks. The DotEnv service reads a gitignored .env file from the app bundle; the API key never enters source control. REST calls use HTTPS. The bundled .appex binary should be verified to match the Swift source in OneSignalWidget/, but that is a trust/supply-chain concern rather than a runtime security issue.
Level of scrutiny
- Public API change:
ILiveActivities.Exitis marked[Obsolete]in the interface and both platform implementations. This is a breaking-change signal to SDK consumers and deserves human sign-off. - CI/CD deletions: Five GitHub Actions workflows (
asana-add-comment,asana-create-task,asana-update-issue,claude-code-review,claude) are removed entirely. Intentional housekeeping or accidental removal should be confirmed. - Pre-built binary:
OneSignalWidgetExtension.appexis committed and bundled — humans should verify it matches the reference Swift source and is the correct build artifact.
Other factors
The three reported bugs are all nits (dotenv quote-stripping, dead-code overwrite, blocking async + narrow exception catch) and do not block correctness for users following the documented .env.example format. However, the combination of public API surface changes, infrastructure deletions, and a committed binary makes this PR too broad for automated approval alone.
fadi-george
changed the title
- Strip surrounding quotes from .env values (standard dotenv convention) - Move _loaded flag to finally block so exceptions don't leave DotEnv permanently broken; broaden catch to Exception - Restructure UpdateLiveActivityAsync as if/else-if to eliminate dead code when eventType is "end" Made-with: Cursor
fadi-george
force-pushed
the
fadi/sdk-4220-dotnet-sdk-live-activities
branch
from
5f481ba to
943d12f
Compare
fadi-george
force-pushed
the
fadi/sdk-4220-dotnet-sdk-live-activities
branch
from
943d12f to
c48cf0e
Compare
|
@claude review |
|
Claude finished @fadi-george's task in 1m 19s —— View job Code Review
Overall this is a well-structured PR that successfully adds Live Activities to the demo app. The implementation is clean and follows existing patterns. There are a few items that need attention before merging. Items Requiring Human Sign-Off1. Workflow deletions — Five GitHub Actions workflows are deleted ( 2. Pre-built binary — Issues Found3. The current implementation has two problems that were partially addressed since the previous review comments, but one remains:
4. Dead code in The 5.
6. START button has no enabled/disabled binding to The START button starts enabled in XAML regardless of whether 7.
Nits
SummaryThe core implementation is solid. The only actionable fix before merge is the async-over-sync call in |
|
|
||
| public void Initialize(AppViewModel viewModel) | ||
| { | ||
| _viewModel = viewModel; | ||
|
|
||
| var hasApiKey = viewModel.HasApiKey(); | ||
| UpdateButton.IsEnabled = hasApiKey; |
There was a problem hiding this comment.
🟡 In LiveActivitiesSection.Initialize(), UpdateButton.IsEnabled and EndButton.IsEnabled are set to hasApiKey without also checking whether a live activity has been started, contradicting the XAML's explicit IsEnabled='False' defaults. With a valid API key in .env, both buttons become enabled at app launch before START is pressed, allowing REST calls to a non-existent activity that silently fail. Fix by also tracking whether StartLiveActivity() has been called before enabling these buttons.
Extended reasoning...
What the bug is and how it manifests
In LiveActivitiesSection.Initialize() (lines 36-42), the code sets UpdateButton.IsEnabled = hasApiKey and EndButton.IsEnabled = hasApiKey. This overrides the XAML's explicit IsEnabled='False' defaults for both buttons. With a valid API key in .env, both UPDATE and END are immediately enabled at app launch before any live activity has been created in the current session.
The specific code path that triggers it
InitializeComponent()runs, loading XAML withIsEnabled='False'on both buttons. TheActivityIdEntrydefault text'order-1'triggersOnActivityIdChanged, but since_viewModelis null at that point,_viewModel?.HasApiKey() == trueevaluates to false, so the buttons stay disabled.Initialize(viewModel)is called.HasApiKey()returns true (valid .env key). Both buttons are unconditionally re-enabled.- User sees UPDATE and END enabled before pressing START. Tapping either sends a
POSTto/apps/{appId}/live_activities/order-1/notificationsfor an activity that does not exist, which returns an HTTP error.UpdateLiveActivityAsyncreturnsfalseand the error is only logged — no UI feedback.
Why existing code doesn't prevent it
Initialize() ignores whether StartLiveActivity() has been called. It has no knowledge of whether an activity ID in use was created in this session. The XAML author explicitly set IsEnabled='False' as the initial state, but Initialize() unconditionally overrides this based solely on API key presence.
Addressing the refutation
The refutation argues this is intentional for multi-session use (updating an activity from a previous session). While plausible as a future design goal, the XAML's explicit IsEnabled='False' defaults are a clear signal that the author intended these buttons to start disabled. If multi-session support were the intent, the XAML would not set IsEnabled='False'. Furthermore, the code has no UI affordance (e.g., instructions, placeholder text, or label) hinting that you can use UPDATE/END for a pre-existing activity — making the enabled state confusing rather than helpful.
Impact
With a valid API key, at app launch both buttons appear active. Pressing END or UPDATE sends a REST call for a non-existent activity, silently returns false, and leaves the buttons still enabled. No crash, no data loss, but the button state does not reflect reality.
Step-by-step proof
- User has a valid
.envfile withONESIGNAL_API_KEY=abc123. - App launches.
DotEnv.Load()reads the key.HasApiKey()returns true. - XAML loads with
UpdateButton.IsEnabled=FalseandEndButton.IsEnabled=False. OnActivityIdChangedfires duringInitializeComponent()with_viewModel == null; both buttons stay disabled (null check evaluates to false).Initialize(viewModel)is called. Lines 40-41:UpdateButton.IsEnabled = true,EndButton.IsEnabled = true.- User sees both buttons enabled without having pressed START.
- User taps END.
EndLiveActivityAsynccallsUpdateLiveActivityAsync("order-1", "end", ...). - HTTP POST to
/apps/{appId}/live_activities/order-1/notificationsfails (no such activity).LogManager.Instance.Elogs the error. Buttons remain enabled.
Fix
Track a _liveActivityStarted boolean set to true in OnStartClicked. In Initialize(), set UpdateButton.IsEnabled = hasApiKey && _liveActivityStarted and EndButton.IsEnabled = hasApiKey && _liveActivityStarted. Update OnActivityIdChanged similarly.
There was a problem hiding this comment.
The refactoring to RefreshUpdateEndButtons() improved the logic (now also checks for non-empty activity ID and IsLiveActivityUpdating), but the core issue is still present: there is no check for whether StartLiveActivity() has been called.
With a valid API key and the default "order-1" activity ID, RefreshUpdateEndButtons() in Initialize() still evaluates to true at app launch — enabling both UPDATE and END before START is pressed.
The fix suggested in the original comment is still needed: add a private bool _liveActivityStarted; field, set it to true in OnStartClicked, and add && _liveActivityStarted to the canInteract check in RefreshUpdateEndButtons().
|
@claude review |
|
Claude finished @fadi-george's task in 1m 37s —— View job Code Review
Good progress since the last round — three of the five issues from the previous review have been resolved. Two remain. Fixed Since Last Review ✅
The async-over-sync call in Still Outstanding1. UPDATE/END buttons enabled at launch before START is pressed ( This was flagged in the previous two review rounds and is still present. Here is the exact flow:
The fix is a one-liner: // Add field
private bool _liveActivityStarted;
// Set in OnStartClicked
private void OnStartClicked(object? sender, EventArgs e)
{
_viewModel?.StartLiveActivity();
_liveActivityStarted = true; // add this
RefreshUpdateEndButtons();
}
// Add to canInteract condition
bool canInteract =
_liveActivityStarted // add this
&& !string.IsNullOrWhiteSpace(ActivityIdEntry.Text)
&& _viewModel?.HasApiKey() == true
&& _viewModel?.IsLiveActivityUpdating != true;Items Requiring Human Sign-Off2. Workflow deletions — Five GitHub Actions workflows are removed ( 3. Pre-built binary — SummaryThe core implementation is solid and earlier issues are resolved. The one remaining code fix is the |
| private void RefreshUpdateEndButtons() | ||
| { | ||
| bool canInteract = | ||
| !string.IsNullOrWhiteSpace(ActivityIdEntry.Text) | ||
| && _viewModel?.HasApiKey() == true | ||
| && _viewModel?.IsLiveActivityUpdating != true; | ||
| UpdateButton.IsEnabled = canInteract; | ||
| EndButton.IsEnabled = canInteract; | ||
| } | ||
|
|
||
| private void OnOrderNumberChanged(object? sender, TextChangedEventArgs e) | ||
| { | ||
| if (_viewModel != null) | ||
| _viewModel.LiveActivityOrderNumber = e.NewTextValue; | ||
| } | ||
|
|
||
| private void OnStartClicked(object? sender, EventArgs e) | ||
| { | ||
| _viewModel?.StartLiveActivity(); | ||
| RefreshUpdateEndButtons(); | ||
| } |
There was a problem hiding this comment.
🟡 The START button's enabled state is never updated when IsLiveActivityUpdating becomes true — RefreshUpdateEndButtons() correctly disables UPDATE and END during an in-flight async call, but StartButton.IsEnabled is only managed in OnActivityIdChanged based on text content alone. A user can tap START while an UPDATE or END REST call is in-flight, causing StartDefault() to be called on the SDK concurrently with an active update/end request for the same activityId. Fix by also incorporating IsLiveActivityUpdating into the START button's enabled condition, either inside RefreshUpdateEndButtons() or as an early-return guard in OnStartClicked.
Extended reasoning...
What the bug is and how it manifests
RefreshUpdateEndButtons() (lines 61–68 of LiveActivitiesSection.xaml.cs) computes a canInteract boolean that checks _viewModel?.IsLiveActivityUpdating \!= true before enabling UPDATE and END. The PropertyChanged handler (lines 47–48) calls this method whenever IsLiveActivityUpdating changes, so both those buttons correctly go disabled the moment an async HTTP call starts. However, the same handler never touches StartButton.IsEnabled. START's only enabled/disabled logic lives in OnActivityIdChanged (line 57), which sets it based purely on whether the text field is non-empty — completely independent of the updating state.
The specific code path that triggers it
- User presses START →
StartLiveActivity()runs, activity is created,RefreshUpdateEndButtons()is called (START remains enabled because text is present). - User presses UPDATE →
UpdateLiveActivityAsync()setsIsLiveActivityUpdating = true, firesPropertyChanged, which callsRefreshUpdateEndButtons()— UPDATE and END are disabled, START is untouched and remains enabled. - While the HTTP call is in-flight, user taps START again →
OnStartClickedruns with noIsLiveActivityUpdatingcheck, calls_viewModel?.StartLiveActivity(), which also has no such guard inAppViewModel. _repository.StartDefaultLiveActivity(activityId, ...)callsOneSignal.LiveActivities.StartDefault()for the sameactivityIdthat already has an active update in progress.
Why existing code does not prevent it
The IsLiveActivityUpdating guard was added to UpdateLiveActivityAsync and EndLiveActivityAsync in AppViewModel (the if (IsLiveActivityUpdating) return; checks at the top of each method), but no equivalent guard was added to StartLiveActivity(). Likewise, RefreshUpdateEndButtons() was written to gate only UPDATE and END, and no corresponding logic was added to gate START. The XAML also does not bind StartButton.IsEnabled to any view-model property.
Impact
If the user taps START during an in-flight update or end call, StartDefault() is invoked on the SDK for an activityId that is mid-update. The SDK may attempt to register a new activity token while simultaneously processing an update token, creating a race condition in the SDK's token management. In practice this is an edge-case timing window in a demo app, and the failure mode is non-fatal, but it is an inconsistency relative to the clearly intentional guards on UPDATE and END — hence nit severity.
Step-by-step proof
ActivityIdEntry.Textis"order-1"(non-empty).StartButton.IsEnabled = true.- User taps START →
OnStartClicked→StartLiveActivity()→StartDefault("order-1", ...). Activity is live. - User taps UPDATE →
OnUpdateClicked→await _viewModel.UpdateLiveActivityAsync()→ setsIsLiveActivityUpdating = true→PropertyChangedfires →RefreshUpdateEndButtons()→UpdateButton.IsEnabled = false,EndButton.IsEnabled = false.StartButton.IsEnabledis not changed; it is stilltrue. - While the HTTP POST is awaited (network I/O), user taps START again.
OnStartClickedruns. NoIsLiveActivityUpdatingcheck.StartLiveActivity()runs. NoIsLiveActivityUpdatingcheck there either.StartDefault("order-1", ...)is called while the previous update call is still in-flight.
How to fix it
Option A — extend RefreshUpdateEndButtons() to also cover START:
private void RefreshUpdateEndButtons()
{
bool isUpdating = _viewModel?.IsLiveActivityUpdating == true;
bool hasText = \!string.IsNullOrWhiteSpace(ActivityIdEntry.Text);
bool canInteract = hasText && _viewModel?.HasApiKey() == true && \!isUpdating;
StartButton.IsEnabled = hasText && \!isUpdating;
UpdateButton.IsEnabled = canInteract;
EndButton.IsEnabled = canInteract;
}Option B — add an early-return guard to OnStartClicked:
private void OnStartClicked(object? sender, EventArgs e)
{
if (_viewModel?.IsLiveActivityUpdating == true) return;
_viewModel?.StartLiveActivity();
RefreshUpdateEndButtons();
}Either option closes the gap consistently with the existing update/end guards.
| /// </summary> | ||
| /// <param name="activityId">The (app-provided) ID of the activity that is being exited.</param> | ||
| /// <returns>Awaitable boolean of whether the operation succeeded or failed</returns> | ||
| [Obsolete("Currently unsupported, avoid using this method.")] |
There was a problem hiding this comment.
🟡 The [Obsolete] message 'Currently unsupported, avoid using this method.' on ILiveActivities.Exit() is factually inaccurate for iOS: iOSLiveActivitiesManager.Exit() is fully implemented and calls OneSignalNative.LiveActivities.Exit() with success/failure callbacks. A more accurate message would be 'Deprecated: use the REST API to end live activities instead.' since the PR's stated intent is to deprecate the method in favor of the REST API path, not to signal that the method does not work.
Extended reasoning...
What the bug is
The [Obsolete] attribute added to Exit(activityId) on the interface (ILiveActivities.cs:22) uses the message 'Currently unsupported, avoid using this method.' This message is applied uniformly to both platform implementations. On Android, the message is accurate — AndroidLiveActivitiesManager.Exit() simply logs a message and returns false, making it a genuine no-op. On iOS, however, the message is factually wrong.
The specific code that contradicts the message
iOSLiveActivitiesManager.Exit() is a fully operational async method:
[Obsolete("Currently unsupported, avoid using this method.")]
public async Task<bool> Exit(string activityId)
{
BooleanCallbackProxy proxy = new BooleanCallbackProxy();
OneSignalNative.LiveActivities.Exit(
activityId,
response => proxy.OnResponse(true),
response => proxy.OnResponse(false)
);
return await proxy;
}It creates a BooleanCallbackProxy, calls the native iOS SDK's LiveActivities.Exit() with success and failure handlers, and awaits the result. This method works correctly on iOS.
Why existing code doesn't prevent the confusion
The [Obsolete] attribute is placed on the interface definition, which is the single source of truth for IDE tooling. When an iOS developer calls Exit() on their ILiveActivitiesManager reference, they will see 'Currently unsupported, avoid using this method.' as a compiler warning. The natural reading of 'Currently unsupported' is 'does not function right now,' not 'being deprecated in favor of another approach.' The word 'Currently' specifically reinforces the interpretation that the method is non-functional at present.
Impact
An iOS developer who reads this warning in their IDE will either: (a) conclude the method doesn't work and stop using it (desired behavior for the wrong reason), or (b) be confused because their calls to Exit() have been working. The PR's actual intent — as stated in the description — is to mark the method as deprecated in favor of the REST API approach, not to indicate the method is broken. The message fails to communicate this intent.
Addressing the refutation
The refutation argues 'unsupported' can reasonably mean 'not supported going forward.' While that interpretation is possible in isolation, the qualifier 'Currently' shifts the phrase toward describing present state rather than future direction. Compare: 'Currently deprecated, prefer the REST API' (intent clear) vs. 'Currently unsupported' (implies it does not function today). Furthermore, the [Obsolete] attribute already signals deprecation by its presence — the message text is an opportunity to explain why, which 'Currently unsupported' fails to do for iOS callers.
Step-by-step proof
- iOS developer has code calling OneSignal.LiveActivities.Exit(activityId).
- After this PR merges, their IDE shows: warning CS0612: 'ILiveActivitiesManager.Exit(string)' is obsolete: 'Currently unsupported, avoid using this method.'
- Developer reads 'Currently unsupported' and concludes Exit() does not work on their platform.
- In reality, Exit() calls the native SDK and returns a meaningful success/failure result.
- A more accurate message: 'Deprecated: use the REST API to end live activities instead.' correctly tells the developer why to migrate without misleading them about current functionality.
How to fix it
Change the [Obsolete] message on ILiveActivities.cs:22 (and the corresponding platform implementations for consistency) from 'Currently unsupported, avoid using this method.' to something like 'Deprecated: use the REST API to end live activities instead. This method may be removed in a future release.'
There was a problem hiding this comment.
The backend api no longer supports this behavior for iOS or wrappers.
2 participants
Description
One Line Summary
Add Live Activities section to the DotNet MAUI demo app with SDK integration, REST API controls, and a pre-built widget extension.
Details
Motivation
The DotNet SDK demo app was missing Live Activities support. Other platform demos (React Native, Flutter) already have this feature. This PR brings parity by adding a full Live Activities section with start/update/end controls.
Scope
Exit(exitLiveActivity) marked as[Obsolete]across the interface and both platform implementationsOneSignalApiServiceOneSignalWidgetExtension.appexbundled inPlatforms/iOS/PlugIns/(Swift source inOneSignalWidget/for reference).envfile support viaDotEnvhelper (reads bundled.envas a MauiAsset)VisualStateManagerdisabled styles toPrimaryButtonStyleandOutlineButtonStyleFiles changed
ILiveActivities.cs,iOSLiveActivitiesManager.cs,AndroidLiveActivitiesManager.cs— deprecateExitmethodLiveActivitiesSection.xaml/.cs— new UI sectionAppViewModel.cs— Live Activity state and commandsOneSignalRepository.cs—StartDefaultLiveActivity,UpdateLiveActivityAsync,HasApiKeyOneSignalApiService.cs—UpdateLiveActivityAsync,DotEnv-based API keyDotEnv.cs— reads.envfrom app bundleMauiProgram.cs—SetupDefaultcall,DotEnv.Load()demo.csproj— bundles.envand widget extension.appexStyles.xaml/Colors.xaml— disabled button visual statesOneSignalWidget/— Swift widget source files (reference)Platforms/iOS/PlugIns/OneSignalWidgetExtension.appex— pre-built widgetTesting
Manual testing
.envfile is read correctly and API key is available.envfileAffected code checklist
Checklist
Overview
Testing
Final pass
Made with Cursor