- Notifications
You must be signed in to change notification settings - Fork4.4k
chore: added static url analytics#41431
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:release
Are you sure you want to change the base?
Conversation
coderabbitaibot commentedNov 28, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
WalkthroughAdds four static URL analytics events, a new Spring utility to emit those events with application/page metadata, and integrates event emission into static URL service flows to send analytics after persistence. Changes
sequenceDiagram participant Client participant StaticUrlServiceImpl participant DB as Database participant AnalyticsUtil as StaticUrlAnalyticsUtils participant SessionUser as SessionUserService participant AnalyticsService Client->>StaticUrlServiceImpl: request static URL operation (enable/disable/update) StaticUrlServiceImpl->>DB: persist changes (save application/page) DB-->>StaticUrlServiceImpl: return persisted entity StaticUrlServiceImpl->>AnalyticsUtil: send*StaticUrlEvent(event, entity, slug[, previousSlug]) AnalyticsUtil->>SessionUser: getCurrentUser() SessionUser-->>AnalyticsUtil: current user context AnalyticsUtil->>AnalyticsService: sendEvent(event, properties) AnalyticsService-->>AnalyticsUtil: acknowledge AnalyticsUtil-->>StaticUrlServiceImpl: return entity StaticUrlServiceImpl-->>Client: return resultEstimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for usingCodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Actionable comments posted: 6
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/AnalyticsEvents.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlAnalyticsUtils.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlServiceImpl.java(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-30T07:15:20.287Z
Learnt from: ashit-rathRepo: appsmithorg/appsmith PR: 41312File: app/client/src/utils/helpers.tsx:1124-1136Timestamp: 2025-10-30T07:15:20.287ZLearning: In app/client/src/utils/helpers.tsx, within the getUpdateRouteForSlugPath function, the pageSlug parameter extracted from the route includes the trailing hyphen (e.g., "home-page-" not "home-page"). Additionally, when the static slug conversion branch is executed, params.basePageId is guaranteed to be defined because the code path requires successful page data fetch from the server as a precondition.Applied to files:
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlServiceImpl.java
🧬 Code graph analysis (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlAnalyticsUtils.java (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitUtils.java (1)
GitUtils(18-202)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: server-unit-tests / server-unit-tests
- GitHub Check: server-spotless / spotless-check
🔇 Additional comments (5)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/AnalyticsEvents.java (1)
99-103:LGTM!New analytics events are well-organized with a descriptive comment and follow the existing naming conventions. The default constructor will produce lowercase event names (e.g.,
static_url_enabled), which is consistent with other events in this enum.app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlServiceImpl.java (1)
70-71:LGTM!The
StaticUrlAnalyticsUtilsdependency is properly injected via constructor (Lombok's@RequiredArgsConstructor).app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlAnalyticsUtils.java (3)
26-33:LGTM!Clean class structure with proper annotations and dependency injection.
95-114:LGTM!Analytics properties are well-structured with proper null handling using
defaultIfNull. The inclusion of git connection status provides useful context.
124-145:LGTM!Page analytics properties are appropriately structured with defensive null checking for nested page name access.
| returnsessionUserService.getCurrentUser().flatMap(user -> { | ||
| Map<String,Object>analyticsProps =buildApplicationAnalyticsProps(application,uniqueSlug); | ||
| log.debug( | ||
| "Sending static URL analytics event: {} for applicationId: {}, slug: {}", | ||
| event.getEventName(), | ||
| application.getId(), | ||
| uniqueSlug); | ||
| returnanalyticsService | ||
| .sendEvent(event.getEventName(),user.getUsername(),analyticsProps) | ||
| .thenReturn(application); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Empty user Mono could cause unexpected behavior.
IfsessionUserService.getCurrentUser() returns an emptyMono, theflatMap won't execute and an emptyMono<Application> is returned. This could cause issues upstream where the caller expects the application to be returned. Consider usingswitchIfEmpty ordefaultIfEmpty to handle this gracefully.
- return sessionUserService.getCurrentUser().flatMap(user -> {+ return sessionUserService.getCurrentUser()+ .flatMap(user -> { Map<String, Object> analyticsProps = buildApplicationAnalyticsProps(application, uniqueSlug); log.debug( "Sending static URL analytics event: {} for applicationId: {}, slug: {}", event.getEventName(), application.getId(), uniqueSlug); return analyticsService .sendEvent(event.getEventName(), user.getUsername(), analyticsProps) .thenReturn(application);- });+ })+ .defaultIfEmpty(application);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| returnsessionUserService.getCurrentUser().flatMap(user -> { | |
| Map<String,Object>analyticsProps =buildApplicationAnalyticsProps(application,uniqueSlug); | |
| log.debug( | |
| "Sending static URL analytics event: {} for applicationId: {}, slug: {}", | |
| event.getEventName(), | |
| application.getId(), | |
| uniqueSlug); | |
| returnanalyticsService | |
| .sendEvent(event.getEventName(),user.getUsername(),analyticsProps) | |
| .thenReturn(application); | |
| }); | |
| returnsessionUserService.getCurrentUser() | |
| .flatMap(user -> { | |
| Map<String,Object>analyticsProps =buildApplicationAnalyticsProps(application,uniqueSlug); | |
| log.debug( | |
| "Sending static URL analytics event: {} for applicationId: {}, slug: {}", | |
| event.getEventName(), | |
| application.getId(), | |
| uniqueSlug); | |
| returnanalyticsService | |
| .sendEvent(event.getEventName(),user.getUsername(),analyticsProps) | |
| .thenReturn(application); | |
| }) | |
| .defaultIfEmpty(application); |
🤖 Prompt for AI Agents
Inapp/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlAnalyticsUtils.javaaround lines 45 to 57, handle the case where sessionUserService.getCurrentUser()returns an empty Mono by ensuring the method still returns the application;change the reactive chain so that after the flatMap you append aswitchIfEmpty(Mono.just(application)) (or equivalent) to skip sending analyticswhen there is no user but still return the application upstream, and optionallyadd a debug log in the switchIfEmpty branch to indicate analytics was skipped.| returnsessionUserService.getCurrentUser().flatMap(user -> { | ||
| Map<String,Object>analyticsProps =buildPageAnalyticsProps(page,applicationId,uniqueSlug); | ||
| log.debug( | ||
| "Sending static URL analytics event: {} for pageId: {}, applicationId: {}, slug: {}", | ||
| event.getEventName(), | ||
| page.getId(), | ||
| applicationId, | ||
| uniqueSlug); | ||
| returnanalyticsService | ||
| .sendEvent(event.getEventName(),user.getUsername(),analyticsProps) | ||
| .thenReturn(page); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Same empty user concern as above.
Apply similardefaultIfEmpty(page) handling to ensure the page is always returned even if no user is present.
🤖 Prompt for AI Agents
Inapp/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlAnalyticsUtils.javaaround lines 72 to 85, the call tosessionUserService.getCurrentUser().flatMap(...) can produce an empty Mono whenno user exists causing the method to return empty; append .defaultIfEmpty(page)after the flatMap chain so the page is always returned when the user is absent(i.e., sessionUserService.getCurrentUser().flatMap(...).defaultIfEmpty(page));no other behavior changes required....server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlServiceImpl.java OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlServiceImpl.java OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlServiceImpl.java OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlServiceImpl.java OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
| returnapplicationService | ||
| .save(appFromDB) | ||
| .flatMap(savedApp ->staticUrlAnalyticsUtils.sendApplicationStaticUrlEvent( | ||
| AnalyticsEvents.STATIC_URL_APP_SLUG_UPDATED,savedApp,normalizedSlug)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Can you capture what was in the previous state, like what was the old stug for page and application?
Failed server tests
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (6)
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlServiceImpl.java (4)
271-273:Analytics failure could still break the primary flow.The
flatMapchain means an analytics failure propagates and fails the entireautoGenerateStaticUrloperation. Add.onErrorResume()to swallow analytics errors.
423-429:Analytics failure could still break the slug update.Same concern as above—wrap with
.onErrorResume()to prevent analytics issues from failing the save.
549-551:Analytics failure could block the delete response.Apply
.onErrorResume()to ensure static URL deletion succeeds even if analytics fails.
716-723:Analytics failure could block page slug updates.Both branches use
flatMapfor analytics. Apply.onErrorResume()to both paths.Also applies to: 748-755
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlAnalyticsUtils.java (2)
59-72:Empty user Mono could cause unexpected behavior.If
sessionUserService.getCurrentUser()returns empty, theflatMapwon't execute and an emptyMono<Application>is returned. Add.defaultIfEmpty(application)after theflatMap.
102-116:Same empty user concern for page events.Apply
.defaultIfEmpty(page)to ensure the page is returned even when no user session exists.
🧹 Nitpick comments (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlServiceImpl.java (1)
735-741:Redundant previous slug capture.The previous slug is already captured at lines 702-706 via
capturedPreviousSlug. This second fetch frompageFromDBre-reads the same value since no modification has occurred yet. Consider reusingcapturedPreviousSluginstead.return pageService.findById(pageId, null).flatMap(pageFromDB -> {- // Capture previous slug from fresh DB fetch for analytics- String prevSlugFromDB = null;- if (pageFromDB.getUnpublishedPage() != null) {- prevSlugFromDB =- pageFromDB.getUnpublishedPage().getUniqueSlug();- }- final String capturedPrevSlugFromDB = prevSlugFromDB;- PageDTO editPage = pageFromDB.getUnpublishedPage(); if (editPage != null) { editPage.setUniqueSlug(normalizedPageSlug); } return pageService .save(pageFromDB) .flatMap(savedPage -> staticUrlAnalyticsUtils.sendPageStaticUrlEvent( AnalyticsEvents.STATIC_URL_PAGE_SLUG_UPDATED, savedPage, applicationId, normalizedPageSlug,- capturedPrevSlugFromDB));+ capturedPreviousSlug)); });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlAnalyticsUtils.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlServiceImpl.java(8 hunks)
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-01-30T10:47:35.449Z
Learnt from: ApekshaBhosaleRepo: appsmithorg/appsmith PR: 38925File: app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java:0-0Timestamp: 2025-01-30T10:47:35.449ZLearning: In reactive programming with Spring WebFlux, avoid using `block()` as it can lead to thread pool exhaustion. Instead, maintain the reactive chain using operators like `flatMap()` and handle empty cases with `defaultIfEmpty()`. When adding tags or metrics, ensure they are applied within the reactive chain.Applied to files:
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlServiceImpl.java
📚 Learning: 2025-10-30T07:15:20.287Z
Learnt from: ashit-rathRepo: appsmithorg/appsmith PR: 41312File: app/client/src/utils/helpers.tsx:1124-1136Timestamp: 2025-10-30T07:15:20.287ZLearning: In app/client/src/utils/helpers.tsx, within the getUpdateRouteForSlugPath function, the pageSlug parameter extracted from the route includes the trailing hyphen (e.g., "home-page-" not "home-page"). Additionally, when the static slug conversion branch is executed, params.basePageId is guaranteed to be defined because the code path requires successful page data fetch from the server as a precondition.Applied to files:
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlServiceImpl.java
📚 Learning: 2025-10-28T03:30:58.299Z
Learnt from: ashit-rathRepo: appsmithorg/appsmith PR: 41312File: app/client/src/sagas/InitSagas.ts:260-271Timestamp: 2025-10-28T03:30:58.299ZLearning: In app/client/src/sagas/InitSagas.ts, when constructing consolidatedApiParams for static page URLs, the code intentionally reuses applicationId and defaultPageId fields to pass staticApplicationSlug and staticPageSlug values respectively. This is by design, even though ConsolidatedApiParams type has dedicated staticApplicationSlug and staticPageSlug fields.Applied to files:
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlServiceImpl.java
📚 Learning: 2025-10-28T09:17:22.519Z
Learnt from: ashit-rathRepo: appsmithorg/appsmith PR: 41312File: app/client/src/pages/AppIDE/components/AppSettings/components/PageSettings.tsx:330-359Timestamp: 2025-10-28T09:17:22.519ZLearning: In the Appsmith codebase, slug normalization for static URLs (application slugs and page slugs) handles lowercase conversion on the backend. The frontend helper `filterAccentedAndSpecialCharacters` in `app/client/src/pages/AppIDE/components/AppSettings/utils.ts` intentionally preserves the user's input casing to avoid modifying the entered value too drastically, as the backend will normalize to lowercase during persistence.Applied to files:
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlServiceImpl.java
📚 Learning: 2025-06-18T10:23:51.839Z
Learnt from: abhvsnRepo: appsmithorg/appsmith PR: 40966File: app/server/reactive-caching/src/main/java/com/appsmith/caching/aspects/DistributedLockAspect.java:139-139Timestamp: 2025-06-18T10:23:51.839ZLearning: In reactive code, using `defaultIfEmpty()` on a Mono ensures it will never be empty, which prevents `block()` from returning null. For example, `instanceIdProvider.getInstanceId().defaultIfEmpty("unknown")` guarantees that the Mono will always emit a value, making subsequent `block()` calls safe from null pointer exceptions.Applied to files:
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlAnalyticsUtils.java
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: sondermanishRepo: appsmithorg/appsmith PR: 30521File: app/server/appsmith-server/src/main/java/com/appsmith/server/exports/exportable/ExportServiceCEImpl.java:0-0Timestamp: 2024-10-08T15:32:34.115ZLearning: The `sanitizeEntities` method in `ExportServiceCEImpl.java` should return `Mono.empty()` instead of `Mono.empty().then()` for simplicity and clarity.Applied to files:
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlAnalyticsUtils.java
📚 Learning: 2024-10-04T05:51:15.138Z
Learnt from: abhvsnRepo: appsmithorg/appsmith PR: 36596File: app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/UserSignupCEImpl.java:475-475Timestamp: 2024-10-04T05:51:15.138ZLearning: In `app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/UserSignupCEImpl.java`, the variable `newsletterSignedUpUserEmail` is always expected to be non-null because the user's email is always present in the user object, so an additional null check is unnecessary.Applied to files:
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlAnalyticsUtils.java
🧬 Code graph analysis (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlAnalyticsUtils.java (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitUtils.java (1)
GitUtils(18-202)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: server-spotless / spotless-check
- GitHub Check: server-unit-tests / server-unit-tests
🔇 Additional comments (6)
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlServiceImpl.java (3)
4-4:LGTM on the new import and field injection.The
AnalyticsEventsimport andStaticUrlAnalyticsUtilsfield are appropriately added for the analytics integration.Also applies to: 70-71
415-420:Good: Previous slug capture for analytics.Capturing the previous slug before modification addresses the reviewer feedback to track the old state.
701-706:Good: Previous slug capture implemented.This addresses the request to track the old slug state for analytics.
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlAnalyticsUtils.java (3)
26-32:LGTM on class structure.Standard Spring component with appropriate dependencies. Using
@RequiredArgsConstructorwith final fields is clean.
127-152:LGTM on application analytics properties.Proper use of
defaultIfNullfor null-safety andGitUtils.isArtifactConnectedToGitfor git status. Conditional inclusion ofpreviousSlugis clean.
163-190:LGTM on page analytics properties.Null-safe extraction of page name and proper use of
defaultIfNull. The absence ofisGitConnectedis reasonable since page-level events don't directly carry git metadata.
Failed server tests
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlServiceImpl.java (2)
563-571:STATIC_URL_DISABLED analytics is correctly chained after full cleanupHooking
STATIC_URL_DISABLEDafter both the application and all pages are saved, and guarding it withonErrorResume, keeps static URL teardown reliable while still recording the event. If you later need to know which slug was disabled in analytics, you could capture the pre-disable slug beforedisableStaticUrlSettings(application)and extend the event payload/schema to include it, but that’s an optional enhancement.
719-785:Page slug analytics now carry old/new values for both clear and set flowsThe
updatePageSlugchanges correctly:
- Capture
applicationIdonce from the resolvedApplication.- Capture the previous slug from the in-memory
pagewhen clearing, and from a fresh DB fetch when setting, soSTATIC_URL_PAGE_SLUG_UPDATEDevents get bothnewSlug(ornullon clear) and a reliablepreviousSlug.- Use
onErrorResumeon both analytics calls so slug updates/clears are never blocked by analytics failures.Using
TextUtils.makeSlugto derivenormalizedPageSlugis also consistent with the backend-driven slug normalization behavior we use elsewhere for static URLs. Based on learnings, this keeps slug casing and normalization semantics aligned across flows.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlServiceImpl.java(8 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-30T07:15:20.287Z
Learnt from: ashit-rathRepo: appsmithorg/appsmith PR: 41312File: app/client/src/utils/helpers.tsx:1124-1136Timestamp: 2025-10-30T07:15:20.287ZLearning: In app/client/src/utils/helpers.tsx, within the getUpdateRouteForSlugPath function, the pageSlug parameter extracted from the route includes the trailing hyphen (e.g., "home-page-" not "home-page"). Additionally, when the static slug conversion branch is executed, params.basePageId is guaranteed to be defined because the code path requires successful page data fetch from the server as a precondition.Applied to files:
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlServiceImpl.java
📚 Learning: 2025-10-28T03:30:58.299Z
Learnt from: ashit-rathRepo: appsmithorg/appsmith PR: 41312File: app/client/src/sagas/InitSagas.ts:260-271Timestamp: 2025-10-28T03:30:58.299ZLearning: In app/client/src/sagas/InitSagas.ts, when constructing consolidatedApiParams for static page URLs, the code intentionally reuses applicationId and defaultPageId fields to pass staticApplicationSlug and staticPageSlug values respectively. This is by design, even though ConsolidatedApiParams type has dedicated staticApplicationSlug and staticPageSlug fields.Applied to files:
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlServiceImpl.java
📚 Learning: 2025-10-28T09:17:22.519Z
Learnt from: ashit-rathRepo: appsmithorg/appsmith PR: 41312File: app/client/src/pages/AppIDE/components/AppSettings/components/PageSettings.tsx:330-359Timestamp: 2025-10-28T09:17:22.519ZLearning: In the Appsmith codebase, slug normalization for static URLs (application slugs and page slugs) handles lowercase conversion on the backend. The frontend helper `filterAccentedAndSpecialCharacters` in `app/client/src/pages/AppIDE/components/AppSettings/utils.ts` intentionally preserves the user's input casing to avoid modifying the entered value too drastically, as the backend will normalize to lowercase during persistence.Applied to files:
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlServiceImpl.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: server-spotless / spotless-check
- GitHub Check: server-unit-tests / server-unit-tests
🔇 Additional comments (3)
app/server/appsmith-server/src/main/java/com/appsmith/server/staticurl/StaticUrlServiceImpl.java (3)
4-70:Analytics dependencies are wired in cleanlyImporting
AnalyticsEventsand injectingStaticUrlAnalyticsUtilsvia the existing@RequiredArgsConstructorpattern aligns with the usages below and keeps this service’s dependencies consistent; no changes needed here.
271-282:STATIC_URL_ENABLED analytics is sequenced correctly and made non-blockingEmitting
STATIC_URL_ENABLEDonly aftergenerateUniquePageSlugsForApplicationcompletes, and swallowing failures withonErrorResume, ensures analytics can’t breakautoGenerateStaticUrlwhile still logging issues. Just confirm thatsendApplicationStaticUrlEventreturns the sameApplicationinstance (savedApp); if not, consider wrapping it withthenReturn(savedApp)so this method’s return value is unambiguous.
424-443:Previous application slug capture for update analytics looks correctCapturing
previousSlugfromappFromDB.getStaticUrlSettings().getUniqueSlug()before mutating, and then sendingSTATIC_URL_APP_SLUG_UPDATEDwith bothnormalizedSlugandcapturedPreviousSlug, gives analytics the expected before/after context without changing the core save flow. TheonErrorResumefallback also keeps slug updates from failing due to analytics issues.
Failed server tests
|
Uh oh!
There was an error while loading.Please reload this page.
Description
Tip
Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team).
Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR.
Fixes #
Issue Numberor
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags=""
🔍 Cypress test results
Warning
Tests have not run on the HEADdcc8e7b yet
Fri, 28 Nov 2025 09:59:02 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.