feat: well designed transactional e-mails - #3375
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a complete email system: React-based email templates and tooling, server-side Thymeleaf rendering and EmailService, Gradle and CI pipeline changes to build/export email assets, refactors email senders to use the service, and updates tests and e2e helpers for the new flow. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as App / Kotlin Sender
participant ES as EmailService
participant TE as EmailTemplateEngine
participant MR as EmailMessageResolver
participant GVP as EmailGlobalVariablesProvider
participant SMTP as JavaMailSender
participant Recipient
Client->>ES: sendEmailTemplate(recipient, template, locale, properties, attachments?)
ES->>GVP: invoke() -> global vars
GVP-->>ES: globalVariables
ES->>TE: render template with context (props + globals)
TE->>MR: resolveMessage / postProcessMessage (ICU + XML fragments)
MR-->>TE: processed message
TE-->>ES: rendered HTML (with th:utext output, newlines -> <br/>)
ES->>ES: extract title/subject
ES->>SMTP: sendEmail(recipient, subject, html, attachments, bcc, replyTo)
SMTP->>Recipient: delivered MIME message (HTML)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.github/workflows/test.yml (1)
373-427: Add validation for email-code-checks in the final gate.The
email-code-checksjob is in theneedslist (line 379) but is not validated in the bash script that checks individual job results. The pipeline could incorrectly report success if email checks fail.📋 Add missing validation check
if [[ "${{ needs.e2e-install-deps.result }}" != "success" ]]; then failed_jobs+=("e2e-install-deps") fi + + if [[ "${{ needs.email-code-checks.result }}" != "success" ]]; then + failed_jobs+=("email-code-checks") + fi if [[ "${#failed_jobs[@]}" -gt 0 ]]; then echo "The following jobs failed: ${failed_jobs[*]}" exit 1 fibackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/V2UserControllerTest.kt (1)
135-155: EnsureneedsEmailVerificationis restored even when the test fails (usetry/finally).
Right now any exception before Line 154 (including insidewaitForNotThrowing) will skip the restore and can poison subsequent tests.Proposed fix
fun `it sends an email when updating user email`() { val oldNeedsVerification = tolgeeProperties.authentication.needsEmailVerification - tolgeeProperties.authentication.needsEmailVerification = true - - val requestDTO = - UserUpdateRequestDto( - email = "ben@ben.aaa", - name = "Ben Ben", - currentPassword = initialPassword, - ) - performAuthPut("/v2/user", requestDTO).andIsOk - - waitForNotThrowing(timeout = 2000, pollTime = 25) { - emailTestUtil.verifyEmailSent() - } - assertThat(emailTestUtil.messageContents.single()) - .contains(tolgeeProperties.frontEndUrl.toString()) - - tolgeeProperties.authentication.needsEmailVerification = oldNeedsVerification + tolgeeProperties.authentication.needsEmailVerification = true + try { + val requestDTO = + UserUpdateRequestDto( + email = "ben@ben.aaa", + name = "Ben Ben", + currentPassword = initialPassword, + ) + performAuthPut("/v2/user", requestDTO).andIsOk + + waitForNotThrowing(timeout = 2000, pollTime = 25) { + emailTestUtil.verifyEmailSent() + } + assertThat(emailTestUtil.messageContents.single()) + .contains(tolgeeProperties.frontEndUrl.toString()) + } finally { + tolgeeProperties.authentication.needsEmailVerification = oldNeedsVerification + } }backend/api/src/main/kotlin/io/tolgee/controllers/resetPassword/ResetPasswordRequestHandler.kt (1)
43-57: Header looks inconsistent with the “Initial password configuration” subject.
IfEmailParams.headeris user-visible, it probably should trackisInitial(or be omitted for initial config).Proposed fix
val params = EmailParams( to = request.email, subject = if (isInitial) "Initial password configuration" else "Password reset", - header = "Password reset", + header = if (isInitial) "Initial password configuration" else "Password reset", text = """ ${if (isInitial) "To set a password for your account, <b>follow this link</b>:<br/>" else "To reset your password, <b>follow this link</b>:<br/>"} <a href="$url">$url</a><br/><br/> If you have not requested this e-mail, please ignore it.<br/><br/> """.trimIndent(), )
🤖 Fix all issues with AI agents
In @.github/actions/download-backend-build/action.yaml:
- Around line 18-33: The tar commands are using the wrong argument order (third
arg treated as archive member) so replace each malformed line that looks like
`tar --zstd -xf <archive> <dest>` with a proper extraction that changes to the
destination directory (use tar -C $DEST_DIR ...) and, if the archive contains a
top-level source directory you want removed, add --strip-components=N to drop
that prefix; ensure each archive is extracted with the correct target subpath
(e.g., use tar --zstd -xf <archive> -C $DEST_DIR backend/api/build or tar --zstd
-xf <archive> --strip-components=N -C $DEST_DIR backend/api/build as
appropriate), computing N from the depth of the source-directory when needed.
In @.github/workflows/release.yml:
- Around line 132-138: The curl -d JSON payload uses single quotes so the shell
will not expand $VERSION; change the payload to allow variable expansion (for
example replace the single-quoted JSON string with a double-quoted string and
escape internal quotes) so that the -d argument becomes something like
"{\"ref\":\"main\",\"inputs\":{\"release-version\":\"$VERSION\"}}", ensuring
$VERSION is interpolated when the curl command in the run block executes.
In
@backend/data/src/main/kotlin/io/tolgee/component/email/EmailVerificationSender.kt:
- Line 18: The code interpolates resultCallbackUrl into the verification link in
EmailVerificationSender (val url = "$resultCallbackUrl/${user.id}/$code") using
an unvalidated value that may come from
EmailVerificationService.getCallbackUrl() when tolgeeProperties.frontEndUrl is
unset; fix by validating or restricting resultCallbackUrl before use: either
require tolgeeProperties.frontEndUrl to be set and throw/return an error if
missing, or implement domain/host whitelist validation on the callbackUrl
returned by EmailVerificationService.getCallbackUrl(); perform the check inside
EmailVerificationSender (or centralize in getCallbackUrl()) and only build the
url if the callback passes validation (reject or canonicalize otherwise) to
eliminate open-redirect/phishing vectors.
In @e2e/docker-compose.yml:
- Around line 14-17: Update the Mailpit image tag in the docker-compose service
definition (the image: axllent/mailpit:v1.27 entry) to a non-vulnerable release;
change it to axllent/mailpit:v1.28.1 or later to remediate CVE-2026-21859, or if
you cannot move off the v1.27 line use axllent/mailpit:v1.27.10 as the minimum
safe patch; after updating the image tag in the docker-compose.yml, pull the new
image and recreate the service (docker-compose pull && docker-compose up -d or
equivalent) to apply the fix.
In @email/.config/tolgeerc.json:
- Around line 1-14: The paths in this tolgeerc.json are relative to the config
file location, so update the "patterns" entries to point up one level (e.g.,
"../emails/**/*.ts?(x)" and "../components/**/*.ts?(x)"), change "extractor" to
"./extractor.ts" (since the extractor lives in the same .config directory), and
update "pull"."path" to "../i18n" so the CLI resolves email/i18n and component
files correctly from the config's location.
In @email/.eslintrc.json:
- Line 26: Replace the permissive ban rule configuration that currently disables
"@typescript-eslint/ban-ts-comment" with a stricter setting: change the rule
value from "off" to ["error", { "ts-ignore": "allow-with-description" }] so that
any use of @ts-ignore requires an inline rationale; update the JSON entry for
"@typescript-eslint/ban-ts-comment" accordingly and run lint to ensure existing
@ts-ignore occurrences include descriptions or are removed.
- Line 34: The ESLint rule "@typescript-eslint/no-non-null-assertion" was turned
off in the email/.eslintrc.json which allows use of the non-null assertion
operator (!) in email template code; re-enable the rule (set it to "error" or
"warn") and then remove or replace existing `!` usages in the email templating
code with safe patterns (optional chaining, explicit null/undefined checks,
default values, or runtime guards) inside template rendering functions and
helpers (search for usages in template renderers, e.g., functions that access
template variables or context); run the linter and update failing files to
handle possible nulls explicitly so runtime template rendering cannot throw due
to hidden non-null assertions.
In @gradle/docker.gradle:
- Around line 53-55: The cleanDocker task currently calls delete(dockerPath) at
configuration time which will remove the docker directory even when the task
isn't run; change the task definition for the task named cleanDocker to defer
deletion to execution by moving the delete(dockerPath) call into a doLast { ...
} action (i.e., replace the top-level delete(dockerPath) with a doLast block
that performs delete(dockerPath)), referencing the existing cleanDocker task and
the dockerPath variable.
In @gradle/utils.gradle:
- Around line 2-29: The problem is that npm is resolved at configuration time
causing build failures and npmCommandName and dockerCommandName have
inconsistent types; change npmCommandName to be lazy like dockerCommandName by
assigning it as a Closure that calls resolveExecutable("npm") (i.e.,
npmCommandName = { resolveExecutable("npm") }) and then update all call sites
that currently use npmCommandName directly to invoke it as a closure
(npmCommandName()) so both npmCommandName and dockerCommandName are the same
type and resolution happens at execution time.
🟡 Minor comments (12)
e2e/cypress/common/apiCalls/common.ts-468-488 (1)
468-488: Add null check for email before accessing its ID.Unlike
getAgencyInvitationLinkswhich properly throws an error when the email is not found,getOrderConfirmationdirectly accessesemail.IDwithout validating thatTypeErrorinstead of a descriptive error.Proposed fix
export const getOrderConfirmation = () => getAllEmails().then((emails) => { const email = emails.find((e) => e.Subject.includes('Your translation order to') ); + if (!email) { + throw new Error('Order confirmation email not found'); + } return getEmail(email.ID).then((e) => {.github/workflows/inactive-issues.yml-13-13 (1)
13-13: Review and test v9 compatibility, particularly the label management change in v7.The version bump from v5 to v9 includes breaking changes that require attention:
- v7 breaking change: The action no longer manages the stale label for items listed in
exempt-issue-labelsorexempt-pr-labels. Your workflow relies on these exemptions to protect important issues and PRs from closure. Verify that items with exempt labels are still handled correctly and not unexpectedly closed.- v9 statefulness: The action is now stateful and may resume from the first unprocessed item if a previous run didn't complete all operations. Monitor logs for "operations-per-run" warnings and consider adjusting the job cadence or increasing
operations-per-runto ensure all items are processed in a single run.- v9 runtime: Node.js 20 is required (no action needed unless custom scripts are added).
Permissions (
issues: write,pull-requests: write) are sufficient for the current configuration.email/package.json-39-40 (1)
39-40: Inconsistent version pinning betweenreactandreact-dom.
reactis pinned to an exact version (19.1.0) whilereact-domuses a caret range (^19.1.0). This inconsistency may lead to version mismatches or peer dependency warnings during installation.🔧 Align version constraints
- "react": "19.1.0", - "react-dom": "^19.1.0", + "react": "^19.1.0", + "react-dom": "^19.1.0",Or if exact pinning is intentional for stability:
- "react": "19.1.0", - "react-dom": "^19.1.0", + "react": "19.1.0", + "react-dom": "19.1.0",email/HACKING.md-1-182 (1)
1-182: Doc polish: fix placeholder/grammar + markdownlint warnings for clarity.Suggested minimal edits (addresses the static-analysis hints):
Proposed doc tweaks
-If you need real world examples, they provide a bunch of great examples based on real-world emails written using +If you need real-world examples, they provide a bunch of great examples based on real-world emails written using -When using styles, make sure to use things that are "email friendly". That means, no flexbox, no grid, and pretty much -anything that's cool in \[CURRENT_YEAR]. +When using styles, make sure to use things that are "email friendly". That means no flexbox, no grid, and pretty much +anything that's considered modern layout CSS. -Be careful, [**SVG images are poorly supported**](https://www.caniemail.com/features/image-svg/) and therefore should +Be careful, [**SVGs are poorly supported**](https://www.caniemail.com/features/image-svg/) and therefore should -How the social icons were generated: -- Get SVG from https://simpleicons.org/ +How the social icons were generated: +- Get SVG from Simple Icons (https://simpleicons.org/)gradle/liquibase.gradle-44-50 (1)
44-50: Module-leveldiffChangeLogtasks don't invoke Liquibase; they only compile Kotlin.The
tasks.register('diffChangeLog') { dependsOn 'compileKotlin' }defined in gradle/liquibase.gradle (lines 48-50) creates a task that doesn't actually generate changelogs. While the root-leveldiffChangeLogtask (in build.gradle) correctly orchestrates Docker, bootRun, and finalizes with the Liquibasediffchangelogtasks, the module-level versions are misleadingly named.If a developer invokes
:data:diffChangeLogdirectly, they'll only compile Kotlin with no changelog output, creating confusion about whether the task works. The module-level tasks should either depend on the Liquibasediffchangelogtask or be removed from this closure.Suggested fix
tasks.named('diff') { dependsOn 'compileKotlin' } -tasks.register('diffChangeLog') { - dependsOn 'compileKotlin' -} +tasks.named('diffChangelog') { + dependsOn 'compileKotlin' +}This makes the Liquibase plugin task depend on compilation, allowing module-level invocation to work as expected.
gradle/webapp.gradle-42-47 (1)
42-47: Directory creation executes at configuration time, not execution time.The
createBuildDirtask body runs during Gradle's configuration phase, not when the task is executed. This means the directory is created even when the task is skipped or not part of the build graph.Proposed fix: Wrap in doLast
tasks.register('createBuildDir') { + doLast { File directory = new File(buildDir as String) if (!directory.exists()) { directory.mkdir() } + } }backend/app/build.gradle-264-275 (1)
264-275: DuplicatebootJarconfiguration detected.The
archiveFileNameandmanifest.attributesare configured twice:
- Lines 252-257: Old
bootJar { ... }block (unchanged)- Lines 268-275: New
tasks.named('bootJar', Jar) { ... }blockThis duplication is redundant. Consider removing one of these blocks to avoid confusion and ensure a single source of truth.
Proposed fix: Remove duplicate configuration
Either remove the old block (lines 252-257) entirely, or remove the duplicate settings from the new named block:
tasks.named('bootJar', Jar) { duplicatesStrategy(DuplicatesStrategy.EXCLUDE) - - archiveFileName = "tolgee-${project.version}.jar" - manifest { - attributes('Implementation-Version': project.version) - } }backend/data/src/main/kotlin/io/tolgee/configuration/tolgee/TolgeeProperties.kt-134-141 (1)
134-141: Doc polish: remove trailing"\n\n"frombackEndUrldescription (unless intentionally used by doc generator).If the doc generator doesn’t need explicit trailing newlines, dropping them avoids odd spacing in rendered config docs.
email/.config/extractor.ts-166-171 (1)
166-171: HandleundefinedfordefaultValue.value.When
defaultValueisundefined(attribute not found), accessing.valuewill throw. The code at line 158-164 adds a warning but doesn't return early, so line 168 could still accessundefined.value.🐛 Proposed fix
const res = processTranslateCall( keyName.value, - defaultValue.value, + defaultValue?.value ?? null, line, code );email/components/ImgResource.ts-49-53 (1)
49-53: MIME type inference may be incorrect for SVG files.The MIME type is constructed as
image/${ext}which works for common formats (png, jpg, gif) but SVG requiresimage/svg+xml, notimage/svg.Suggested fix
} else { const blob = readFileSync(file); const ext = extname(file).slice(1); - newProps.src = `data:image/${ext};base64,${blob.toString('base64')}`; + const mime = ext === 'svg' ? 'image/svg+xml' : `image/${ext}`; + newProps.src = `data:${mime};base64,${blob.toString('base64')}`; }backend/data/src/main/kotlin/io/tolgee/email/EmailMessageResolver.kt-59-76 (1)
59-76: Stack-based tag matching may throw on malformed input.If an ICU message contains a closing tag without a matching opening tag (e.g.,
</link>without<link>),stack.removeLast()at line 72 will throwNoSuchElementException. While malformed ICU messages shouldn't occur in practice, defensive handling could prevent cryptic errors.Suggested defensive fix
} else { // Closing tag - stack.removeLast() + if (stack.size > 1) stack.removeLast() // Keep at least the root element "</th:block></th:block>" }email/components/If.ts-68-69 (1)
68-69: Potential runtime issue: accessingchildren[1]when only one child is provided.When
demoValue === falseand only a single child is provided (no else branch),children[1]will beundefined, returningundefinedfrom the component. This may cause unexpected behavior or render nothing without warning.Consider adding a fallback or warning:
Proposed fix
- if (demoValue === false) return children[1]; + if (demoValue === false) return children[1] ?? null;Alternatively, log a warning when
demoValue === falsebut no else branch is provided, to help developers catch misconfigurations during preview.
🧹 Nitpick comments (32)
email/.gitignore (1)
3-3: Use explicit scope for theoutdirectory pattern.The pattern
outmatchesoutdirectories at any depth. Use/outto explicitly target the directory at the email root only, following.gitignorebest practices.- out + /outemail/.eslintrc.json (2)
6-6: Reconsider the*.jsignore pattern—it may be too broad.The ignore pattern
"*.js"will exclude all JavaScript files from linting, including configuration files, build outputs, and potentially legitimate JS sources in the email package. Consider being more specific with patterns like"dist/**","build/**", or"*.generated.js"to avoid silently ignoring unintended files.
24-24: Consider enforcing stricter console logging in production code.The
no-consolerule is set to"warn"rather than"error", which allows console statements to reach production code. While useful during development, consider"error"for production builds or add environment-specific overrides (dev: warn, prod: error) to prevent accidental logging in transactional email rendering.e2e/cypress/e2e/notifications/notifications.cy.ts (1)
26-33: LGTM! Consider using Cypress's built-in assertions for better integration.The improved error messages with expected and actual values are excellent for debugging test failures.
However, consider refactoring to use Cypress's
expect()instead of bareassert()for better integration with Cypress's retry logic and error reporting.♻️ Optional refactor to use Cypress assertions
function assertNewestEmail( expectedSubject: string, expectedTextFragment: string ) { getLastEmail().then(({ subject, html }) => { - assert( - subject === expectedSubject, - 'Subject does not match, expected: ' + - expectedSubject + - ', actual: ' + - subject - ); - assert(html.includes(expectedTextFragment), 'Mail does not contain text'); + expect(subject, 'Subject does not match').to.equal(expectedSubject); + expect(html, 'Mail does not contain text').to.include(expectedTextFragment); }); }Note: Cypress's
expect()provides better error messages automatically (showing expected vs actual) and integrates with Cypress's command retry logic.e2e/cypress/common/apiCalls/common.ts (1)
550-554: Add type annotation for theidparameter.The
idparameter lacks a type annotation. Based on thestring.Proposed fix
-export const getEmail = (id) => +export const getEmail = (id: string) => cy .request({ url: `http://localhost:21080/api/v1/message/${id}` }) .then((r) => r.body as EmailSummary);backend/data/src/main/kotlin/io/tolgee/configuration/HibernateConfig.kt (1)
25-31: Consider adding documentation explaining the hibernate-trace profile usage.The Hypersistence Utils dependency is correctly configured and exists in
settings.gradle. The newHibernateConfig.ktdoesn't duplicate existing functionality—HibernateConfiguration.kthandles production activity tracking, whileHibernateConfig.ktprovides opt-in query debugging via the profile guard. The implementation is correct and follows good practices.Adding KDoc to explain when developers should activate the "hibernate-trace" profile would improve discoverability:
/** * Enables Hibernate query stack trace logging when the "hibernate-trace" profile is active. * Useful for debugging N+1 queries and identifying query origins during development. * * Activate with: `--spring.profiles.active=hibernate-trace` */ @Component @Profile("hibernate-trace") class HibernateConfig : HibernatePropertiesCustomizer { override fun customize(hibernateProperties: MutableMap<String, Any>) { hibernateProperties[AvailableSettings.STATEMENT_INSPECTOR] = QueryStackTraceLogger("io.tolgee") } }email/package.json (1)
22-22: Consider updating@react-email/componentsto the latest stable version.Version 0.1.0 is an early release; the latest stable version is 0.5.2. While 0.1.0 is compatible with react-email 4.0.16, the official React Email documentation recommends keeping
@react-email/componentsandreact-emailversions in sync. Consider upgrading to the latest version for compatibility and access to bug fixes..github/workflows/release.yml (1)
58-59: Inconsistent formatting of secrets references.Lines 58-59 use
${{secrets...}}without spaces, while other occurrences (lines 50-51, 97-98, 115) use${{ secrets... }}with spaces. Consider aligning for consistency.email/env.d.ts (1)
19-24: LGTM! TypeScript support for Thymeleaf attributes correctly implemented.The module augmentation properly enables type-safe usage of Thymeleaf template attributes (
th:*anddata-th-*) in React JSX. Usingunknownfor the values is appropriate since Thymeleaf attributes accept various dynamic types at runtime.📝 Optional: Add JSDoc for clarity
Consider adding a brief JSDoc comment explaining the purpose of these attributes for developers unfamiliar with Thymeleaf:
declare module 'react' { + /** + * Augments React attributes to support Thymeleaf template attributes. + * - `th:*` attributes: Standard Thymeleaf template syntax + * - `data-th-*` attributes: Alternative Thymeleaf attribute form + */ interface Attributes { [k: `th:${string}`]: unknown; [k: `data-th-${string}`]: unknown; } }gradle/utils.gradle (1)
6-24: Add a timeout to avoid hanging onwhich/where.Not common, but
proc.waitFor()(Line 8) can hang indefinitely. ConsiderwaitForOrKill(...)or a timed wait.email/components/translate.ts (1)
98-101: Potential React element mutation issue.Spreading an object with
{ ...e, key: i }wheneis a React element creates a shallow copy, but this pattern can be fragile. React elements should typically be cloned usingReact.cloneElementto preserve internal invariants.Suggested improvement
if (Array.isArray(formatted)) { return formatted.map((e: string | boolean | object, i) => - typeof e === 'object' ? { ...e, key: i } : e + typeof e === 'object' && React.isValidElement(e) + ? React.cloneElement(e, { key: i }) + : e ) as any; }gradle/e2e.gradle (1)
110-127: Consider using try-with-resources pattern for stream handling.The
outStreamis created indoFirstand closed indoLast, but if the exec command throws an exception beforedoLastruns, the stream may not be properly closed. While Gradle's task lifecycle typically handles this, using a more defensive pattern would be safer.Alternative approach
tasks.register('saveServerLogs', Exec) { group = 'e2e' workingDir = file(E2E_DIR) def outFile = file("$E2E_DIR/server.log") - def outStream = null - doFirst { - outFile.parentFile.mkdirs() - outStream = new FileOutputStream(outFile) - standardOutput = outStream - errorOutput = outStream - } commandLine dockerCommandName(), "compose", "logs" ignoreExitValue = true - doLast { - try { outStream?.close() } catch (ignored) {} - } + doFirst { + outFile.parentFile.mkdirs() + standardOutput = new FileOutputStream(outFile) + errorOutput = standardOutput + } finalizedBy "stopDockerE2e" }Note: Gradle's Exec task should handle stream cleanup when the task completes, so the explicit close may not be necessary.
gradle/webapp.gradle (1)
33-40: Consider excludingnode_modulesfrom inputs.
inputs.dir(webappPath)includesnode_modules/, which can cause unnecessary cache invalidation and rebuilds when dependencies change. The dependencies are already tracked viainstallWebappDeps.Suggested improvement
tasks.register('buildWebapp', Exec) { onlyIf { System.getenv("SKIP_WEBAPP_BUILD") != "true" } commandLine npmCommandName, "run", "build" workingDir = webappPath - inputs.dir(webappPath) + inputs.dir(webappPath).exclude("node_modules/**", "dist/**") outputs.dir("${webappPath}/dist/") dependsOn "installLibraryDeps", "installWebappDeps", "updateStaticTranslations" }gradle/docker.gradle (1)
20-28: Consider usingdockerCommandName()for consistency.The
dockeranddockerBuildx*tasks use hardcoded"docker"command, whilee2e.gradleusesdockerCommandName()fromutils.gradle. For cross-platform consistency, consider using the centralized command resolver.Suggested change for docker task
tasks.register('docker') { doLast { exec { workingDir dockerPath - commandLine "docker", "build", ".", "-t", "tolgee/tolgee", "--cache-from", "type=registry,ref=tolgee/tolgee:latest" + commandLine dockerCommandName(), "build", ".", "-t", "tolgee/tolgee", "--cache-from", "type=registry,ref=tolgee/tolgee:latest" } } dependsOn("dockerPrepare") }Similarly for
createDockerBuildxTask:- def commandParams = ["docker", "buildx", "build", ".", "-t", project.property('dockerImageTag'), "--platform", "linux/arm64,linux/amd64"] + def commandParams = [dockerCommandName(), "buildx", "build", ".", "-t", project.property('dockerImageTag'), "--platform", "linux/arm64,linux/amd64"]Also applies to: 30-48
build.gradle (2)
140-145: Eager configuration of bootRun task.The
project(':server-app').bootRun.systemProperty(...)calls eagerly configure the bootRun task duringdiffChangeLogconfiguration. This should use lazy task configuration to avoid resolving bootRun before it's needed.Suggested improvement
doFirst { - project(':server-app').bootRun - .systemProperty('spring.profiles.active', 'dbschema') - // Use an unlikely-to-be-used port - .systemProperty('server.port', '61987') + project(':server-app').tasks.named('bootRun') { + systemProperty('spring.profiles.active', 'dbschema') + // Use an unlikely-to-be-used port + systemProperty('server.port', '61987') + } }
119-138: Consider idiomaticfinalizedByusage.The
finalizedBylist manipulation pattern works but is non-idiomatic. Gradle'sfinalizedBytypically accepts varargs directly. The current approach with list mutation is harder to read.Alternative approach
tasks.register('diffChangeLog') { project(':server-app').tasks.named("bootRun") { mustRunAfter(startDbChangelogContainer) } - finalizedBy = [ - startDbChangelogContainer, - ':server-app:bootRun', - ':data:diffChangelog', - ] - - if (gradle.ext.billingAppDirectory.exists()) { - finalizedBy.add(':billing-app:diffChangelog') - } - - if (gradle.ext.eeAppDirectoryExists) { - finalizedBy.add(':ee-app:diffChangelog') - } - - finalizedBy.add(stopDbChangelogContainer) + def finalizerTasks = [ + startDbChangelogContainer, + ':server-app:bootRun', + ':data:diffChangelog', + ] + if (gradle.ext.billingAppDirectory.exists()) { + finalizerTasks.add(':billing-app:diffChangelog') + } + if (gradle.ext.eeAppDirectoryExists) { + finalizerTasks.add(':ee-app:diffChangelog') + } + finalizerTasks.add(stopDbChangelogContainer) + finalizedBy finalizerTasksbackend/app/src/test/kotlin/io/tolgee/controllers/resetPassword/ResetPasswordControllerTest.kt (2)
39-46: Good: make email-content assertions eventually-consistent instead of flaky.
Consider extracting the(timeout=2000, pollTime=25)into a shared constant/helper if this pattern is now standard across tests.
48-57: Good: same eventual-consistency fix applied to the “no frontend url” case.
Same minor note about consolidating timeout/poll defaults to avoid drift.email/components/atoms/TolgeeButton.ts (1)
17-29: Minor readability improvement: simplify redundant key-value syntaxThe className prop is correctly supported and preserved by @react-email/components Button. The shorthand can be applied for clarity:
Optional tweak
- return React.createElement(Button, { ...props, className: className }); + return React.createElement(Button, { ...props, className });backend/data/src/main/kotlin/io/tolgee/service/EmailVerificationService.kt (1)
61-65: Consider sending verification email after commit (side-effect inside@Transactional).These calls now pass
UserAccount(good), but they still trigger email sending insidecreateForUser()’s transaction. If the transaction rolls back after the send, users can receive a link/code that never committed. If feasible, publish an event and send via@TransactionalEventListener(phase = AFTER_COMMIT)(or similar).email/components/atoms/TolgeeLink.ts (1)
17-28: Optional: simplify prop handling (avoidReact.createElement+ doubleclassNamehandling).Current logic is fine; this is just a readability refactor.
Proposed refactor
import * as React from 'react'; import { Link, LinkProps } from '@react-email/components'; const LINK_CLASSES = 'text-brand underline'; export default function TolgeeLink(props: LinkProps) { - const className = props.className - ? `${LINK_CLASSES} ${props.className}` - : LINK_CLASSES; - - return React.createElement(Link, { ...props, className: className }); + const { className, ...rest } = props; + const mergedClassName = className ? `${LINK_CLASSES} ${className}` : LINK_CLASSES; + return <Link {...rest} className={mergedClassName} />; }backend/data/src/test/kotlin/io/tolgee/email/EmailTemplateTestConfig.kt (1)
10-23: Test determinism: consider disabling system-locale fallback (avoid env-dependent message resolution).If
ICUReloadableResourceBundleMessageSourcecan fall back to the JVM/system locale, tests may vary by CI environment. Consider explicitly disabling system-locale fallback (and/or “use code as default message”) if supported by this class.email/emails/default.tsx (1)
40-43: Minor: Inconsistent greeting punctuation.The defaultValue
"Hello {recipientName}! 👋,"has both an exclamation mark and a trailing comma, which reads oddly. Compare withregistration-confirm.tsxline 39 which uses"Hello {username},". Consider aligning the formatting for consistency.backend/data/src/main/kotlin/io/tolgee/component/email/TolgeeEmailSender.kt (1)
12-16: Consider simplifying the properties map construction.The chained
.let { if ... }pattern is verbose. A more idiomatic approach:♻️ Suggested refactor
- val properties = - mapOf<String, Any>() - .let { if (params.text != null) it.plus("content" to params.text!!) else it } - .let { if (params.header != null) it.plus("header" to params.header!!) else it } - .let { if (params.recipientName != null) it.plus("recipientName" to params.recipientName!!) else it } + val properties = buildMap<String, Any> { + params.text?.let { put("content", it) } + params.header?.let { put("header", it) } + params.recipientName?.let { put("recipientName", it) } + }email/.config/extractor.ts (2)
45-56: RenametoStringto avoid shadowing the global.Per static analysis, this function shadows the global
toStringproperty. Consider renaming tonodeToStringorextractStringValue.♻️ Suggested rename
-function toString(node: Expression | JSXAttrValue): string | null { +function nodeToString(node: Expression | JSXAttrValue): string | null {Update all call sites accordingly (lines 67, 68, etc.).
117-122: Add bounds checking fornode.argumentsaccess.The code assumes
t()calls always have at least two arguments. While based on learnings the API contract requires both arguments, defensive coding would prevent crashes on malformed input during extraction.♻️ Suggested fix
+ if (node.arguments.length < 2) { + warnings.push({ + warning: 'Found t() call with fewer than 2 arguments', + line: spanToLine(code, node.span), + }); + return; + } const res = processTranslateCall( node.arguments[0].expression, node.arguments[1].expression, line, code );gradle/email.gradle (2)
21-25: Consider declaring inputs for Copy tasks.The
copyEmailResourcestask doesn't declare inputs explicitly. While Gradle's Copy task infers inputs fromfrom, explicitly declaring them can improve build cache reliability.Suggested improvement
tasks.register('copyEmailResources', Copy) { + inputs.dir("${emailPath}/resources") from "${emailPath}/resources" // Not a typo: static so Spring picks it up, then static so the url is /static/emails into "${project.projectDir}/build/generated/resources/main/static/static/emails" }
58-71: Add i18n directory as input to buildEmails task.The Tolgee CLI configuration pulls translation files into
${emailPath}/i18n, which are used via the<LocalizedText>component andt()function during email export. Without this directory as an input, changes to translation files won't trigger a rebuild. Addinputs.dir("${emailPath}/i18n")to detect translation changes.backend/data/src/test/kotlin/io/tolgee/email/EmailServiceTest.kt (1)
158-164: Helper assumes specific MIME structure.The
assertContents()helper navigates a nestedMimeMultipartstructure. If the email format changes (e.g., plain text fallback is added/removed), this will break. Consider adding a comment documenting the expected structure.email/components/If.ts (1)
72-78: Props type should includechildrenfor proper typing.
IfThenandIfElseacceptReact.Attributesbut don't includechildrenin the type, even though they're expected to wrap child content. This may cause TypeScript complaints when using these components with children.Proposed fix
-function IfThen(props: React.Attributes) { +function IfThen(props: React.PropsWithChildren<React.Attributes>) { return React.createElement(React.Fragment, props); } -function IfElse(props: React.Attributes) { +function IfElse(props: React.PropsWithChildren<React.Attributes>) { return React.createElement(React.Fragment, props); }backend/data/src/main/kotlin/io/tolgee/email/EmailService.kt (2)
69-69: Variable shadowing:subjectparameter is shadowed by local declaration.The parameter
subject(line 55) is shadowed by the localval subjecton line 69. While this works, it can be confusing to readers. Consider using a different name for clarity.Proposed fix
- val subject = subject ?: extractEmailTitle(html) - sendEmail(recipient, subject, html, attachments, bcc, replyTo) + val emailSubject = subject ?: extractEmailTitle(html) + sendEmail(recipient, emailSubject, html, attachments, bcc, replyTo)
82-83: Use standard charset name "UTF-8" instead of "UTF8".The IANA-registered charset name is
"UTF-8"(with hyphen). While"UTF8"may work in some contexts, using the standard name is more portable and explicit.Proposed fix
- val helper = MimeMessageHelper(message, MimeMessageHelper.MULTIPART_MODE_MIXED_RELATED, "UTF8") + val helper = MimeMessageHelper(message, MimeMessageHelper.MULTIPART_MODE_MIXED_RELATED, "UTF-8")
| isSignUp: Boolean = true, | ||
| ) { | ||
| val url = "$resultCallbackUrl/$userId/$code" | ||
| val url = "$resultCallbackUrl/${user.id}/$code" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'sendEmailVerification' --type=kt -A 5 | head -100Repository: tolgee/tolgee-platform
Length of output: 92
🏁 Script executed:
# Find the EmailVerificationSender class definition to understand resultCallbackUrl
rg -n 'class EmailVerificationSender' --type=kt -A 20Repository: tolgee/tolgee-platform
Length of output: 92
🏁 Script executed:
# Search for where resultCallbackUrl is passed or set
rg -n 'resultCallbackUrl' --type=kt -B 3 -A 3Repository: tolgee/tolgee-platform
Length of output: 92
🏁 Script executed:
rg -n 'sendEmailVerification' -A 5 | head -100Repository: tolgee/tolgee-platform
Length of output: 5881
🏁 Script executed:
rg -n 'class EmailVerificationSender' -A 20Repository: tolgee/tolgee-platform
Length of output: 2492
🏁 Script executed:
rg -n 'resultCallbackUrl' -B 3 -A 3Repository: tolgee/tolgee-platform
Length of output: 4484
🏁 Script executed:
# Check if there's any URL validation in EmailVerificationService
rg -n 'isValidUrl|validateUrl|URL|isValidCallbackUrl' backend/data/src/main/kotlin/io/tolgee/service/EmailVerificationService.ktRepository: tolgee/tolgee-platform
Length of output: 131
🏁 Script executed:
# Check the full context around how callbackUrl is used
sed -n '159,170p' backend/data/src/main/kotlin/io/tolgee/service/EmailVerificationService.ktRepository: tolgee/tolgee-platform
Length of output: 387
Implement URL validation for resultCallbackUrl to prevent open redirect vulnerabilities.
The resultCallbackUrl can be user-controlled: when tolgeeProperties.frontEndUrl is not configured, the system falls back to the user-provided callbackUrl parameter (from EmailVerificationService.getCallbackUrl() at line 160). This user input is never validated and is directly interpolated into the email body and href at line 18. An attacker could pass callbackUrl=https://attacker.com to generate phishing emails. Either enforce that frontEndUrl is always configured, or validate that user-provided callbackUrl matches an allowed domain list.
🤖 Prompt for AI Agents
In
@backend/data/src/main/kotlin/io/tolgee/component/email/EmailVerificationSender.kt
at line 18, The code interpolates resultCallbackUrl into the verification link
in EmailVerificationSender (val url = "$resultCallbackUrl/${user.id}/$code")
using an unvalidated value that may come from
EmailVerificationService.getCallbackUrl() when tolgeeProperties.frontEndUrl is
unset; fix by validating or restricting resultCallbackUrl before use: either
require tolgeeProperties.frontEndUrl to be set and throw/return an error if
missing, or implement domain/host whitelist validation on the callbackUrl
returned by EmailVerificationService.getCallbackUrl(); perform the check inside
EmailVerificationSender (or centralize in getCallbackUrl()) and only build the
url if the callback passes validation (reject or canonicalize otherwise) to
eliminate open-redirect/phishing vectors.
8707014 to
6e0c9a1
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/test.yml (1)
373-427: Missingemail-code-checksresult validation in the final check script.The
email-code-checksjob is added to theneedsarray (line 379), but the script that validates job results doesn't include a check forneeds.email-code-checks.result. This means a failingemail-code-checksjob won't be reported in the failure message, though it will still block the workflow due to theneedsdependency.Suggested fix
Add a check for
email-code-checksin the script:if [[ "${{ needs.backend-code-checks.result }}" != "success" ]]; then failed_jobs+=("backend-code-checks") fi + if [[ "${{ needs.email-code-checks.result }}" != "success" ]]; then + failed_jobs+=("email-code-checks") + fi + if [[ "${{ needs.frontend-build.result }}" != "success" ]]; then failed_jobs+=("frontend-build") fibackend/app/build.gradle (1)
252-257: DuplicatebootJarconfiguration detected.
bootJaris configured twice:
- Lines 252-257: Using legacy direct configuration
- Lines 268-275: Using modern
tasks.named('bootJar', Jar)patternBoth set the same
archiveFileNameand manifest attributes. Remove the first block to avoid redundancy and maintain consistency with the modern task registration pattern used elsewhere.Suggested fix
-bootJar { - archiveFileName = "tolgee-${project.version}.jar" - manifest { - attributes('Implementation-Version': project.version) - } -} - sourceSets {Also applies to: 268-275
🤖 Fix all issues with AI agents
In @build.gradle:
- Around line 148-152: The bug is that tasks.matching inside subprojects.forEach
references the current script project (':server-app') rather than each
subproject; update the loop to target each subproject's tasks (e.g., use
it.tasks.matching or project(it.path).tasks.matching) and call configureEach {
mustRunAfter ':server-app:bootRun' } on those task providers so each
subproject's diffChangelog task (matching 'diffChangelog') gets the mustRunAfter
dependency instead of repeatedly modifying :server-app's tasks.
In @e2e/cypress/common/apiCalls/common.ts:
- Around line 403-430: The two branches of getParsedEmailVerificationByIndex
extract the verification link with different regexes (index===0 uses replace
with /.*(http:\/\/[\w:/]*).*/gs from getLatestEmail, while the else branch uses
match(/href="([^"]+)"/i) from getEmail), causing inconsistent results; pick one
robust extraction strategy (preferably the href="..." match to capture full link
including https) and apply it to both branches: update the index===0 branch to
extract verifyEmailLink using the same href regex (or unify both to a helper
function like extractVerifyLink(html) called in both places) so
getParsedEmailVerificationByIndex returns consistent verifyEmailLink values.
- Around line 468-488: The getOrderConfirmation function can throw when no
matching email is found because it accesses email.ID without a null check;
update getOrderConfirmation to check that the result of emails.find(...) (email)
is not undefined before calling getEmail(email.ID) and if missing either return
a clear rejected promise or throw a descriptive error (e.g., "Order confirmation
email not found") or return a safe empty object; ensure you reference
getAllEmails and getEmail in the same flow and mirror the
null-check/early-return behavior used in getAgencyInvitationLinks so callers
don't get a confusing runtime exception.
In @gradle/webapp.gradle:
- Around line 42-47: The createBuildDir task currently creates the directory
during Gradle's configuration phase because the File and mkdir() calls are at
the top level of the task registration; move the directory creation into the
task's execution phase by adding an action (e.g., doLast or doFirst) inside the
tasks.register('createBuildDir') block and perform directory creation there (use
buildDir and mkdirs() to ensure parent dirs are created); keep the task
registration intact and only perform filesystem operations inside the action so
the directory is created only when the task actually runs.
🧹 Nitpick comments (21)
e2e/cypress/e2e/notifications/notifications.cy.ts (1)
26-33: Inconsistent error message detail in html assertion.The subject assertion (lines 26-32) helpfully includes both expected and actual values, but the html assertion on line 33 only states "Mail does not contain text" without indicating which text fragment was expected. For consistency and easier debugging, consider including the expected text fragment in the error message.
Suggested improvement
- assert(html.includes(expectedTextFragment), 'Mail does not contain text'); + assert( + html.includes(expectedTextFragment), + 'Mail does not contain text: ' + expectedTextFragment + );e2e/cypress/common/apiCalls/common.ts (2)
490-502: Consider adding proper types forToandFromfields.The
anytypes reduce type safety. Based on usage patterns (e.g.,r.From.Address,r.To[0].Address), these could be more precisely typed.💡 Suggested types
+type EmailAddress = { + Address: string; +}; + type Email = { ID: string; - To: any; - From: any; + To: EmailAddress[]; + From: EmailAddress; Subject: string; }; type EmailSummary = { HTML: string; Subject: string; - To: any; - From: any; + To: EmailAddress[]; + From: EmailAddress; };
550-553: Add type annotation foridparameter.The
idparameter lacks a type annotation. Based on thestring.💡 Suggested fix
-export const getEmail = (id) => +export const getEmail = (id: string) => cy .request({ url: `http://localhost:21080/api/v1/message/${id}` }) .then((r) => r.body as EmailSummary);.github/workflows/release.yml (1)
52-53: Inconsistent secret reference formatting.The formatting normalization is incomplete. Lines 52-53, 80-81, and 98 use spaces inside the braces (
${{ secrets.* }}), but lines 64-65 still use the old compact format (${{secrets.*}}).Suggested fix for consistency
- name: Prepare for docker build if: ${{ steps.version.outputs.VERSION != '' }} run: ./gradlew dockerPrepare env: VERSION: ${{ steps.version.outputs.VERSION }} - TOLGEE_API_KEY: ${{secrets.TOLGEE_API_KEY}} - TOLGEE_API_URL: ${{secrets.TOLGEE_API_URL}} + TOLGEE_API_KEY: ${{ secrets.TOLGEE_API_KEY }} + TOLGEE_API_URL: ${{ secrets.TOLGEE_API_URL }}Also applies to: 64-65, 80-81, 98-98
backend/data/src/main/kotlin/io/tolgee/configuration/tolgee/TolgeeProperties.kt (1)
134-140: LGTM! NewbackEndUrlconfiguration property.The new property is well-documented and correctly allows the backend API URL to be configured separately from
frontEndUrlwhen needed. The nullable design withnulldefault enables appropriate fallback behavior.One minor observation: the description string has a trailing
\n\non line 138 that appears unnecessary, but this doesn't affect functionality.gradle/utils.gradle (1)
27-28: Consider making npm resolution lazy like docker.
npmCommandNameis resolved eagerly during configuration phase, meaning any build will fail if npm isn't installed—even for tasks that don't need it. In contrast,dockerCommandNameis a lazy closure. If this is intentional (npm is always required), consider adding a comment. Otherwise, make it lazy:- npmCommandName = resolveExecutable("npm") + npmCommandName = { resolveExecutable("npm") }Note: This would require updating all usages from
npmCommandNametonpmCommandName().email/.config/extractor.ts (1)
45-56: RenametoStringto avoid shadowing the global.Per static analysis, this shadows the global
toStringproperty, which can cause confusion. Consider renaming to something more descriptive likeextractStringValueornodeToString.Suggested fix
-function toString(node: Expression | JSXAttrValue): string | null { +function extractStringValue(node: Expression | JSXAttrValue): string | null { switch (node.type) { case 'StringLiteral': return node.value; case 'JSXText': return node.value.trim(); case 'JSXExpressionContainer': - return toString(node.expression); + return extractStringValue(node.expression); } return null; }Also update the calls at lines 67, 68, and 89.
email/HACKING.md (2)
12-17: Consider using descriptive link text for accessibility.Links with text like "here" are less accessible and harder to scan. Consider rewriting to include descriptive text:
-If you need real world examples, they provide a bunch of great examples based on real-world emails written using -React Email [here](https://demo.react.email/preview/newsletters/stack-overflow-tips). +If you need real-world examples, they provide [real-world email examples](https://demo.react.email/preview/newsletters/stack-overflow-tips). -They also provide a handful of components [here](https://react.email/components) +They also provide a [component library](https://react.email/components).
176-182: Use markdown link syntax for the bare URL.Per markdown best practices, wrap the URL in proper link syntax:
How the social icons were generated: -- Get SVG from https://simpleicons.org/ +- Get SVG from [Simple Icons](https://simpleicons.org/)email/package.json (1)
39-41: Inconsistent version pinning between react and react-dom.
reactis pinned to an exact version (19.1.0) whilereact-domuses a caret range (^19.1.0). For consistency and to avoid potential mismatches, consider aligning the pinning strategy:"react": "19.1.0", - "react-dom": "^19.1.0", + "react-dom": "19.1.0", "react-email": "4.0.16",backend/data/src/main/kotlin/io/tolgee/component/email/TolgeeEmailSender.kt (1)
12-22: Consider usingbuildMapfor cleaner property construction.The chained
.let{}pattern is verbose and requires!!operators. UsingbuildMapis more idiomatic Kotlin:♻️ Suggested refactor
- val properties = - mapOf<String, Any>() - .let { if (params.text != null) it.plus("content" to params.text!!) else it } - .let { if (params.header != null) it.plus("header" to params.header!!) else it } - .let { if (params.recipientName != null) it.plus("recipientName" to params.recipientName!!) else it } + val properties = buildMap<String, Any> { + params.text?.let { put("content", it) } + params.header?.let { put("header", it) } + params.recipientName?.let { put("recipientName", it) } + } emailService.sendEmailTemplate(email/components/layouts/LayoutCore.tsx (1)
36-38: Conditional attribute spread includesnullvalue.When
thTextisnull, the spread{...{ 'th:text': thText }}will setth:text={null}on the<title>element. Depending on how React Email renders this, it may emit an empty or undefined attribute. Consider conditionally spreading only whenthTextis truthy:Suggested improvement
- <title {...{ 'th:text': thText }}>{convert(subjectPlain)}</title> + <title {...(thText ? { 'th:text': thText } : {})}>{convert(subjectPlain)}</title>backend/data/src/main/kotlin/io/tolgee/email/EmailTemplateConfig.kt (1)
51-62: Clarify the bidirectional wiring pattern.The
messageResolverbean receives thetemplateEngineand assigns itself back to the engine (lines 59-60). While this works to break the circular dependency via thelateinitproperty inEmailTemplateEngine, consider adding a brief comment explaining this bidirectional wiring for future maintainers.Optional documentation
val resolver = EmailMessageResolver(messageResolver, templateEngine) + // Bidirectional wiring: resolver needs engine for fragment rendering, + // engine needs resolver for message resolution during template processing templateEngine.emailMessageResolver = resolverbackend/data/src/test/kotlin/io/tolgee/email/EmailServiceTest.kt (2)
64-67: Mock returns same MimeMessage instance for all calls.The
thenReturn(msg)returns the sameMimeMessageinstance for every call tocreateMimeMessage(). This works because each test method only sends one email, but if a test were to send multiple emails, they would share the same message object, causing unexpected behavior.For robustness, consider using
thenAnswerto return a fresh instance each time:More robust mock setup
- whenever(mailSender.createMimeMessage()).let { - val msg = sender.createMimeMessage() - it.thenReturn(msg) - } + whenever(mailSender.createMimeMessage()).thenAnswer { + sender.createMimeMessage() + }
158-164: Consider adding error handling to the content extraction helper.The
assertContents()helper uses unsafe casts that will throwClassCastExceptionif the MIME structure differs from expectations. While acceptable for tests, a descriptive error message would aid debugging:Optional improvement
private fun MimeMessage.assertContents(): AbstractStringAssert<*> { - return this.content - .let { it as MimeMultipart } - .let { it.getBodyPart(0).content as MimeMultipart } - .let { it.getBodyPart(0).content as String } - .assert + val outer = this.content as? MimeMultipart + ?: error("Expected MimeMultipart as outer content, got ${this.content::class}") + val inner = outer.getBodyPart(0).content as? MimeMultipart + ?: error("Expected MimeMultipart as inner content") + return (inner.getBodyPart(0).content as String).assert }backend/data/src/main/kotlin/io/tolgee/email/EmailService.kt (3)
73-98: Consider removing@AsyncfromsendEmailor making it a private helper.Since
sendEmailTemplatealready has@Asyncand callssendEmaildirectly (line 70), the@Asyncannotation onsendEmailwon't take effect for that call path—Spring's@Asynconly works through the proxy, not for internal method calls within the same bean.If
sendEmailis intended to be called externally as an async method, this is fine as-is. However, if it's primarily an internal helper, the annotation is misleading. Consider either:
- Removing
@AsyncfromsendEmailif it's only called internally, or- Documenting that external callers get async behavior while internal calls are synchronous
82-84: Minor: Use standard charset nameUTF-8.While
UTF8typically works,UTF-8is the standard IANA charset name and is more portable across different implementations.Suggested fix
- val helper = MimeMessageHelper(message, MimeMessageHelper.MULTIPART_MODE_MIXED_RELATED, "UTF8") + val helper = MimeMessageHelper(message, MimeMessageHelper.MULTIPART_MODE_MIXED_RELATED, "UTF-8")
100-107: Title extraction regex may fail on multiline titles.The current regex
<title>(.+?)</title>uses.which doesn't match newlines by default. If a template has a multiline<title>tag (e.g., with Thymeleaf expressions that span lines), this will fail to extract the title.Suggested fix for multiline support
companion object { - private val REGEX_TITLE = Regex("<title>(.+?)</title>") + private val REGEX_TITLE = Regex("<title>(.+?)</title>", RegexOption.DOT_MATCHES_ALL) }email/components/translate.ts (1)
56-58: Consider adding explicit typing to improve maintainability.The
interleavefunction usesany[]types. While this works, adding generic typing would improve type safety and code clarity.Optional typed version
-const interleave = (arr: any[], thing: any): any[] => - ([] as any[]).concat(...arr.map((n) => [n, thing])).slice(0, -1); +const interleave = <T, U>(arr: T[], thing: U): (T | U)[] => + ([] as (T | U)[]).concat(...arr.map((n) => [n, thing])).slice(0, -1);gradle/e2e.gradle (1)
123-125: Empty catch block silently swallows exceptions.The empty catch block could hide file handle issues. Consider logging or at minimum using a proper exception type.
Suggested fix
doLast { - try { outStream?.close() } catch (ignored) {} + try { outStream?.close() } catch (Exception ignored) { /* Stream cleanup, ignore */ } }build.gradle (1)
140-145: Inconsistent task access pattern.Line 141 accesses
bootRundirectly on the project, which is inconsistent with thetasks.namedpattern used elsewhere in this refactoring. While this works, usingtasks.namedwould be more consistent and follows Gradle's recommended lazy configuration approach.Suggested fix
doFirst { - project(':server-app').bootRun - .systemProperty('spring.profiles.active', 'dbschema') - // Use an unlikely-to-be-used port - .systemProperty('server.port', '61987') + project(':server-app').tasks.named('bootRun').configure { + systemProperty('spring.profiles.active', 'dbschema') + // Use an unlikely-to-be-used port + systemProperty('server.port', '61987') + } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
email/package-lock.jsonis excluded by!**/package-lock.jsonemail/resources/facebook.pngis excluded by!**/*.pngemail/resources/github.pngis excluded by!**/*.pngemail/resources/linkedin.pngis excluded by!**/*.pngemail/resources/slack.pngis excluded by!**/*.pngemail/resources/tolgee_logo_text.pngis excluded by!**/*.pngemail/resources/twitter-x.pngis excluded by!**/*.pngemail/resources/twitter.pngis excluded by!**/*.png
📒 Files selected for processing (82)
.github/actions/download-backend-build/action.yaml.github/actions/upload-backend-build/action.yaml.github/workflows/release.yml.github/workflows/reportIntermittentTests.yml.github/workflows/test.yml.gitmodulesDEVELOPMENT.mdbackend/api/build.gradlebackend/api/src/main/kotlin/io/tolgee/controllers/resetPassword/ResetPasswordRequestHandler.ktbackend/app/build.gradlebackend/app/src/main/kotlin/io/tolgee/Application.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/V2UserControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/organizationController/OrganizationControllerInvitingTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ProjectsController/ProjectsControllerInvitationTest.ktbackend/app/src/test/kotlin/io/tolgee/controllers/resetPassword/ResetPasswordControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/service/notification/NotificationServiceTest.ktbackend/data/build.gradlebackend/data/src/main/kotlin/io/tolgee/component/email/EmailVerificationSender.ktbackend/data/src/main/kotlin/io/tolgee/component/email/InvitationEmailSender.ktbackend/data/src/main/kotlin/io/tolgee/component/email/TolgeeEmailSender.ktbackend/data/src/main/kotlin/io/tolgee/configuration/HibernateConfig.ktbackend/data/src/main/kotlin/io/tolgee/configuration/tolgee/TolgeeProperties.ktbackend/data/src/main/kotlin/io/tolgee/dtos/misc/EmailParams.ktbackend/data/src/main/kotlin/io/tolgee/email/EmailGlobalVariablesProvider.ktbackend/data/src/main/kotlin/io/tolgee/email/EmailMessageResolver.ktbackend/data/src/main/kotlin/io/tolgee/email/EmailService.ktbackend/data/src/main/kotlin/io/tolgee/email/EmailTemplateConfig.ktbackend/data/src/main/kotlin/io/tolgee/email/EmailTemplateEngine.ktbackend/data/src/main/kotlin/io/tolgee/service/EmailVerificationService.ktbackend/data/src/main/resources/I18n_en.propertiesbackend/data/src/test/kotlin/io/tolgee/email/EmailGlobalVariablesProviderTest.ktbackend/data/src/test/kotlin/io/tolgee/email/EmailServiceTest.ktbackend/data/src/test/kotlin/io/tolgee/email/EmailTemplateTestConfig.ktbackend/data/src/test/resources/email-i18n-test/messages_en.propertiesbackend/development/build.gradlebackend/ktlint/build.gradlebackend/misc/build.gradlebackend/security/build.gradlebackend/testing/build.gradlebackend/testing/src/main/kotlin/io/tolgee/fixtures/EmailTestUtil.ktbuild.gradlee2e/cypress/common/apiCalls/common.tse2e/cypress/e2e/notifications/notifications.cy.tse2e/cypress/e2e/organizations/organizationInvitations.cy.tse2e/docker-compose.ymlee/backend/app/build.gradleee/backend/tests/build.gradleee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/task/TaskControllerTest.ktemail/.config/extractor.tsemail/.config/tolgeerc.jsonemail/.eslintrc.jsonemail/.gitignoreemail/.prettierrc.jsonemail/HACKING.mdemail/components/For.tsemail/components/If.tsemail/components/ImgResource.tsemail/components/LocalizedText.tsemail/components/Var.tsemail/components/atoms/TolgeeButton.tsemail/components/atoms/TolgeeLink.tsemail/components/layouts/ClassicLayout.tsxemail/components/layouts/LayoutCore.tsxemail/components/parts/.gitkeepemail/components/translate.tsemail/emails/__tests__/test-email.tsxemail/emails/default.tsxemail/emails/registration-confirm.tsxemail/env.d.tsemail/i18n/messages_en.propertiesemail/package.jsonemail/tailwind.config.tsemail/tsconfig.jsongradle.propertiesgradle/docker.gradlegradle/e2e.gradlegradle/email.gradlegradle/liquibase.gradlegradle/utils.gradlegradle/webapp.gradlesettings.gradlewebapp/.tolgeerc.json
💤 Files with no reviewable changes (2)
- .gitmodules
- backend/data/src/main/kotlin/io/tolgee/component/email/InvitationEmailSender.kt
🚧 Files skipped from review as they are similar to previous changes (34)
- e2e/cypress/e2e/organizations/organizationInvitations.cy.ts
- .github/actions/download-backend-build/action.yaml
- email/emails/default.tsx
- email/components/atoms/TolgeeButton.ts
- backend/data/src/main/kotlin/io/tolgee/service/EmailVerificationService.kt
- .github/workflows/reportIntermittentTests.yml
- email/.gitignore
- backend/api/src/main/kotlin/io/tolgee/controllers/resetPassword/ResetPasswordRequestHandler.kt
- email/env.d.ts
- backend/app/src/test/kotlin/io/tolgee/controllers/resetPassword/ResetPasswordControllerTest.kt
- email/i18n/messages_en.properties
- email/tsconfig.json
- email/components/ImgResource.ts
- webapp/.tolgeerc.json
- backend/data/src/main/kotlin/io/tolgee/email/EmailMessageResolver.kt
- email/components/LocalizedText.ts
- email/components/layouts/ClassicLayout.tsx
- backend/misc/build.gradle
- email/components/atoms/TolgeeLink.ts
- email/components/For.ts
- ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/task/TaskControllerTest.kt
- backend/data/src/test/kotlin/io/tolgee/email/EmailGlobalVariablesProviderTest.kt
- backend/data/src/test/resources/email-i18n-test/messages_en.properties
- email/emails/registration-confirm.tsx
- backend/ktlint/build.gradle
- email/.config/tolgeerc.json
- backend/development/build.gradle
- email/.eslintrc.json
- backend/data/src/main/resources/I18n_en.properties
- gradle/docker.gradle
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/V2UserControllerTest.kt
- gradle.properties
- email/.prettierrc.json
- backend/data/src/main/kotlin/io/tolgee/email/EmailGlobalVariablesProvider.kt
🧰 Additional context used
📓 Path-based instructions (5)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: After modifying JPA entities, run./gradlew diffChangeLogto generate Liquibase changelog entries (add--no-daemonflag if docker command is not found)
Run specific test suites instead of the bare test task; use:data:test,:server-app:runStandardTests,:server-app:runContextRecreatingTests,:server-app:runWebsocketTests,:server-app:runWithoutEeTests,:ee-test:test, or:security:testwith the--testsflag for individual tests
Always run./gradlew ktlintFormatbefore commits in backend code
Files:
backend/data/src/main/kotlin/io/tolgee/configuration/HibernateConfig.ktbackend/data/src/main/kotlin/io/tolgee/component/email/TolgeeEmailSender.ktbackend/testing/src/main/kotlin/io/tolgee/fixtures/EmailTestUtil.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ProjectsController/ProjectsControllerInvitationTest.ktbackend/data/src/test/kotlin/io/tolgee/email/EmailServiceTest.ktbackend/data/src/main/kotlin/io/tolgee/configuration/tolgee/TolgeeProperties.ktbackend/app/src/main/kotlin/io/tolgee/Application.ktbackend/data/src/main/kotlin/io/tolgee/component/email/EmailVerificationSender.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/organizationController/OrganizationControllerInvitingTest.ktbackend/data/src/main/kotlin/io/tolgee/dtos/misc/EmailParams.ktbackend/data/src/main/kotlin/io/tolgee/email/EmailTemplateEngine.ktbackend/data/src/main/kotlin/io/tolgee/email/EmailTemplateConfig.ktbackend/data/src/main/kotlin/io/tolgee/email/EmailService.ktbackend/app/src/test/kotlin/io/tolgee/service/notification/NotificationServiceTest.ktbackend/data/src/test/kotlin/io/tolgee/email/EmailTemplateTestConfig.kt
**/*.{ts,tsx,js,jsx,kt,kts}
⚙️ CodeRabbit configuration file
As part of review, please check if the file follows 'The Stepdown Rule': The most important, high-level concepts should be at the top (like a newspaper headline and opening paragraph). Details should increase as you read downward. Functions should be ordered so that a caller appears before the functions it calls. When working with JavaScript components, we allow the main component to live at the bottom of the file as an exception to the rule.
Files:
backend/data/src/main/kotlin/io/tolgee/configuration/HibernateConfig.kte2e/cypress/e2e/notifications/notifications.cy.tsemail/emails/__tests__/test-email.tsxbackend/data/src/main/kotlin/io/tolgee/component/email/TolgeeEmailSender.ktemail/tailwind.config.tsbackend/testing/src/main/kotlin/io/tolgee/fixtures/EmailTestUtil.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ProjectsController/ProjectsControllerInvitationTest.ktbackend/data/src/test/kotlin/io/tolgee/email/EmailServiceTest.ktbackend/data/src/main/kotlin/io/tolgee/configuration/tolgee/TolgeeProperties.ktbackend/app/src/main/kotlin/io/tolgee/Application.ktemail/components/If.tsbackend/data/src/main/kotlin/io/tolgee/component/email/EmailVerificationSender.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/organizationController/OrganizationControllerInvitingTest.ktbackend/data/src/main/kotlin/io/tolgee/dtos/misc/EmailParams.ktemail/components/Var.tsbackend/data/src/main/kotlin/io/tolgee/email/EmailTemplateEngine.ktemail/components/translate.tse2e/cypress/common/apiCalls/common.tsbackend/data/src/main/kotlin/io/tolgee/email/EmailTemplateConfig.ktbackend/data/src/main/kotlin/io/tolgee/email/EmailService.ktbackend/app/src/test/kotlin/io/tolgee/service/notification/NotificationServiceTest.ktbackend/data/src/test/kotlin/io/tolgee/email/EmailTemplateTestConfig.ktemail/components/layouts/LayoutCore.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use Tolgee custom TypeScript path aliases (tg.component/*,tg.service/*,tg.hooks/*,tg.views/*,tg.globalContext/*) instead of relative imports
STRICTLY usedata-cyattributes for component selectors in E2E tests, never text content; use typed helpersgcy('...')orcy.gcy('...')
Files:
e2e/cypress/e2e/notifications/notifications.cy.tsemail/emails/__tests__/test-email.tsxemail/tailwind.config.tsemail/components/If.tsemail/components/Var.tsemail/components/translate.tse2e/cypress/common/apiCalls/common.tsemail/components/layouts/LayoutCore.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: After backend API changes, regenerate TypeScript types by runningnpm run schemafor main API ornpm run billing-schemafor billing API in the webapp directory (backend must be running first)
Use typed React Query hooks fromuseQueryApi.tsfor API communication instead of raw React Query; structure queries withurl,method, andpathparameters, and mutations withinvalidatePrefix
Use Tolgee-specific hooksuseReportEventanduseReportOncefromtg.hooks/useReportEventfor business event tracking and analytics
Files:
e2e/cypress/e2e/notifications/notifications.cy.tsemail/emails/__tests__/test-email.tsxemail/tailwind.config.tsemail/components/If.tsemail/components/Var.tsemail/components/translate.tse2e/cypress/common/apiCalls/common.tsemail/components/layouts/LayoutCore.tsx
e2e/**/*.{ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
e2e/**/*.{ts,js}: UsegenerateStandard()method for E2E test data generation, not the outdatedgenerate()pattern
Backend error codes are automatically converted to lowercase when sent to frontend; use lowercase error code values in intercepts and error handling
Files:
e2e/cypress/e2e/notifications/notifications.cy.tse2e/cypress/common/apiCalls/common.ts
🧠 Learnings (16)
📓 Common learnings
Learnt from: cyyynthia
Repo: tolgee/tolgee-platform PR: 1987
File: email/components/translate.ts:36-37
Timestamp: 2025-06-16T20:31:22.217Z
Learning: In email template processing systems like React Email with Thymeleaf integration, build-time processes that run with NODE_ENV === 'production' are short-lived and terminate after completion, so memory accumulation in data structures like Sets is not a concern as the memory is automatically freed when the process ends.
📚 Learning: 2026-01-09T10:00:46.985Z
Learnt from: CR
Repo: tolgee/tolgee-platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T10:00:46.985Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use Tolgee custom TypeScript path aliases (`tg.component/*`, `tg.service/*`, `tg.hooks/*`, `tg.views/*`, `tg.globalContext/*`) instead of relative imports
Applied to files:
email/package.json
📚 Learning: 2025-06-16T20:27:09.537Z
Learnt from: cyyynthia
Repo: tolgee/tolgee-platform PR: 1987
File: email/package.json:38-41
Timestamp: 2025-06-16T20:27:09.537Z
Learning: Node.js v16.9.0+ supports wildcard subpath patterns in the "exports" field of package.json. Patterns like "./components/*.ts": "./components/*.ts" are valid syntax and are the recommended approach for exposing multiple files. This replaced the deprecated folder export syntax.
Applied to files:
email/package.json
📚 Learning: 2026-01-09T10:00:46.985Z
Learnt from: CR
Repo: tolgee/tolgee-platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T10:00:46.985Z
Learning: Applies to **/*.{ts,tsx} : Use Tolgee-specific hooks `useReportEvent` and `useReportOnce` from `tg.hooks/useReportEvent` for business event tracking and analytics
Applied to files:
email/package.json
📚 Learning: 2026-01-09T10:00:46.985Z
Learnt from: CR
Repo: tolgee/tolgee-platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T10:00:46.985Z
Learning: Applies to **/*.kt : After modifying JPA entities, run `./gradlew diffChangeLog` to generate Liquibase changelog entries (add `--no-daemon` flag if docker command is not found)
Applied to files:
backend/api/build.gradlegradle/liquibase.gradlebackend/data/build.gradlebuild.gradlebackend/app/build.gradle
📚 Learning: 2025-06-16T20:58:15.906Z
Learnt from: cyyynthia
Repo: tolgee/tolgee-platform PR: 1987
File: email/components/For.ts:25-38
Timestamp: 2025-06-16T20:58:15.906Z
Learning: In the Tolgee email template system, the For component intentionally uses React.ReactElement type to restrict children to a single child for easier cloning, and uses Math.random() for keys in development mode following a "quick and dirty" approach that's good enough for development-only workloads. The maintainer prefers KISS (Keep It Simple, Stupid) principle over perfect code quality for development-only components.
Applied to files:
email/emails/__tests__/test-email.tsxemail/components/translate.tsemail/components/layouts/LayoutCore.tsxemail/HACKING.md
📚 Learning: 2026-01-09T10:00:46.985Z
Learnt from: CR
Repo: tolgee/tolgee-platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T10:00:46.985Z
Learning: Applies to **/backend/**/*.test.kt : Use TestData classes for test setup in backend tests, following the pattern: create a TestData class, inject TestDataService, initialize in BeforeEach, and cleanup in AfterEach
Applied to files:
backend/testing/src/main/kotlin/io/tolgee/fixtures/EmailTestUtil.ktbackend/data/src/test/kotlin/io/tolgee/email/EmailServiceTest.ktbuild.gradle
📚 Learning: 2026-01-09T10:00:46.985Z
Learnt from: CR
Repo: tolgee/tolgee-platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T10:00:46.985Z
Learning: Create E2E test data with 3 components: TestData class in `backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/`, E2E Data Controller in `backend/development/src/main/kotlin/io/tolgee/controllers/internal/e2eData/`, and frontend test data object in `e2e/cypress/common/apiCalls/testData/testData.ts`
Applied to files:
backend/testing/src/main/kotlin/io/tolgee/fixtures/EmailTestUtil.ktbackend/data/src/test/kotlin/io/tolgee/email/EmailServiceTest.kt
📚 Learning: 2025-11-06T21:13:09.616Z
Learnt from: cyyynthia
Repo: tolgee/tolgee-platform PR: 3035
File: .github/workflows/release.yml:69-72
Timestamp: 2025-11-06T21:13:09.616Z
Learning: When reviewing CI/CD workflows that reference build system tasks (e.g., gradle tasks like `dockerPrepare`), always examine what those tasks do before flagging path or file existence issues. Build tasks often prepare directories and copy files as part of their execution.
Applied to files:
gradle/utils.gradle.github/workflows/release.ymlgradle/e2e.gradlebuild.gradlegradle/webapp.gradle.github/workflows/test.yml
📚 Learning: 2026-01-09T10:00:46.985Z
Learnt from: CR
Repo: tolgee/tolgee-platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T10:00:46.985Z
Learning: Applies to **/backend/**/*.test.kt : Use `.andAssertThatJson` for asserting API response JSON in backend tests instead of manual assertions
Applied to files:
backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ProjectsController/ProjectsControllerInvitationTest.kt
📚 Learning: 2026-01-09T10:00:46.985Z
Learnt from: CR
Repo: tolgee/tolgee-platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T10:00:46.985Z
Learning: Applies to **/*.kt : Always run `./gradlew ktlintFormat` before commits in backend code
Applied to files:
gradle/liquibase.gradlebackend/data/build.gradlebuild.gradlebackend/security/build.gradle.github/workflows/test.ymlee/backend/tests/build.gradle
📚 Learning: 2025-06-16T20:29:21.117Z
Learnt from: cyyynthia
Repo: tolgee/tolgee-platform PR: 1987
File: email/.config/extractor.ts:114-124
Timestamp: 2025-06-16T20:29:21.117Z
Learning: The t() function in email/.../translate.ts requires both keyName and defaultValue arguments by design. The Tolgee CLI extractor assumes valid usage and does not need to handle cases where the second argument is missing, as such code would be invalid according to the API contract.
Applied to files:
email/.config/extractor.tsemail/components/translate.ts
📚 Learning: 2025-05-09T01:35:14.224Z
Learnt from: laz-001
Repo: tolgee/tolgee-platform PR: 0
File: :0-0
Timestamp: 2025-05-09T01:35:14.224Z
Learning: Changes to PostgreSQL container names, ports, Liquibase changelog paths, and test data directories in the Tolgee platform are often made to provide dedicated resources to parallel test groups (like `ee-test:test` and `server-app:test`), preventing resource congestion and collisions that could lead to deadlocks during testing.
Applied to files:
e2e/docker-compose.ymlbuild.gradle
📚 Learning: 2025-06-11T16:17:35.696Z
Learnt from: cyyynthia
Repo: tolgee/tolgee-platform PR: 3126
File: backend/testing/build.gradle:78-78
Timestamp: 2025-06-11T16:17:35.696Z
Learning: The Gradle module `backend/testing` serves as an internal test pseudo-library; therefore, test-scope dependencies such as `org.springframework.boot:spring-boot-starter-test` are intentionally declared with `implementation`, as the module is only consumed on the test classpath and is not part of production artifacts.
Applied to files:
backend/data/build.gradleee/backend/app/build.gradlebackend/testing/build.gradleee/backend/tests/build.gradlebackend/app/build.gradle
📚 Learning: 2026-01-09T10:00:46.985Z
Learnt from: CR
Repo: tolgee/tolgee-platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T10:00:46.985Z
Learning: Applies to **/*.kt : Run specific test suites instead of the bare test task; use `:data:test`, `:server-app:runStandardTests`, `:server-app:runContextRecreatingTests`, `:server-app:runWebsocketTests`, `:server-app:runWithoutEeTests`, `:ee-test:test`, or `:security:test` with the `--tests` flag for individual tests
Applied to files:
backend/data/build.gradlebuild.gradlebackend/security/build.gradleee/backend/tests/build.gradlebackend/app/build.gradle
📚 Learning: 2025-06-16T20:31:22.217Z
Learnt from: cyyynthia
Repo: tolgee/tolgee-platform PR: 1987
File: email/components/translate.ts:36-37
Timestamp: 2025-06-16T20:31:22.217Z
Learning: In email template processing systems like React Email with Thymeleaf integration, build-time processes that run with NODE_ENV === 'production' are short-lived and terminate after completion, so memory accumulation in data structures like Sets is not a concern as the memory is automatically freed when the process ends.
Applied to files:
email/HACKING.md
🧬 Code graph analysis (4)
e2e/cypress/e2e/notifications/notifications.cy.ts (1)
e2e/cypress/common/apiCalls/common.ts (1)
getAllEmails(514-514)
email/emails/__tests__/test-email.tsx (4)
email/components/Var.ts (1)
Var(25-39)email/components/atoms/TolgeeLink.ts (1)
TolgeeLink(22-28)email/components/If.ts (1)
If(25-70)email/components/For.ts (1)
For(25-39)
backend/data/src/main/kotlin/io/tolgee/email/EmailService.kt (1)
backend/data/src/main/kotlin/io/tolgee/component/email/TolgeeEmailSender.kt (1)
sendEmail(11-27)
email/components/layouts/LayoutCore.tsx (1)
email/components/translate.ts (1)
TranslatedText(24-28)
🪛 Biome (2.1.2)
email/.config/extractor.ts
[error] 45-45: Do not shadow the global "toString" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🪛 LanguageTool
email/HACKING.md
[grammar] ~13-~13: Use a hyphen to join words.
Context: ...ail/docs/introduction). If you need real world examples, they provide a bunch of ...
(QB_NEW_EN_HYPHEN)
[style] ~142-~142: This phrase is redundant (‘G’ stands for ‘graphic’). Use simply “SVGs”.
Context: ...ile you want to insert. Be careful, [SVG images are poorly supported](https://www.can...
(ACRONYM_TAUTOLOGY)
[style] ~145-~145: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...PG, and GIF should be good. It is also very important that files are never deleted, and p...
(EN_WEAK_ADJECTIVE)
🪛 markdownlint-cli2 (0.18.1)
email/HACKING.md
14-14: Link text should be descriptive
(MD059, descriptive-link-text)
16-16: Link text should be descriptive
(MD059, descriptive-link-text)
178-178: Bare URL used
(MD034, no-bare-urls)
⏰ 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). (3)
- GitHub Check: Frontend static check 🪲
- GitHub Check: Build backend 🏗️
- GitHub Check: Build frontend 🏗️
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/task/TaskControllerTest.kt (1)
387-387: Inconsistent email assertion pattern may cause flaky tests.Lines 387 and 403 assert email notifications immediately without
waitForNotThrowing, while the test at lines 109-113 wraps the same type of assertion with a wait. If email delivery is now async, these assertions could be flaky.Proposed fix
- assertThat(notificationUtil.newestEmailNotification()).contains("has been canceled") + waitForNotThrowing(timeout = 2000, pollTime = 25) { + assertThat(notificationUtil.newestEmailNotification()).contains("has been canceled") + }- assertThat(notificationUtil.newestEmailNotification()).contains("has been finished") + waitForNotThrowing(timeout = 2000, pollTime = 25) { + assertThat(notificationUtil.newestEmailNotification()).contains("has been finished") + }Also applies to: 403-403
🤖 Fix all issues with AI agents
In
@backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translationSuggestionController/TranslationSuggestionWithCacheTest.kt:
- Line 23: Replace uses of the deprecated
org.springframework.boot.test.mock.mockito.@MockBean in
TranslationSuggestionWithCacheTest (and other occurrences around lines 39-43)
with org.springframework.test.context.bean.override.mockito.@MockitoBean: remove
the MockBean import and add the MockitoBean import, update every @MockBean
annotation on fields in the test class to @MockitoBean, and run the tests to
ensure the test class structure (e.g., any @Configuration usage) is compatible
with the behavioral differences of @MockitoBean.
In @backend/app/src/test/kotlin/io/tolgee/AuthProviderChangeTest.kt:
- Line 23: Replace the deprecated @MockBean usage in AuthProviderChangeTest with
@MockitoBean by changing the import from
org.springframework.boot.test.mock.mockito.MockBean to
org.springframework.test.context.bean.override.mockito.MockitoBean and update
the annotation on the mocked field accordingly; also remove the redundant
@Autowired on that field (the MockitoBean is auto-registered and injected by the
context).
In @backend/app/src/test/kotlin/io/tolgee/automation/AutomationCachingTest.kt:
- Line 20: Replace the deprecated @SpyBean import with MockitoSpyBean: change
the import from org.springframework.boot.test.mock.mockito.SpyBean to
org.springframework.test.context.bean.override.mockito.MockitoSpyBean and update
any annotation usages of @SpyBean in AutomationCachingTest (and other test
classes if present) to @MockitoSpyBean so tests use the supported Spring Boot
3.4+ spy annotation.
In @backend/app/src/test/kotlin/io/tolgee/batch/AbstractBatchJobsGeneralTest.kt:
- Line 19: The test uses the deprecated @SpyBean import and annotations; replace
the import of org.springframework.boot.test.mock.mockito.SpyBean with
org.springframework.test.context.bean.override.mockito.MockitoSpyBean and update
every @SpyBean annotation in AbstractBatchJobsGeneralTest (and any usages in
this file) to @MockitoSpyBean so the code uses the Spring Boot 3.4-compatible
MockitoSpyBean.
In @backend/app/src/test/kotlin/io/tolgee/cache/AbstractCacheTest.kt:
- Around line 30-31: Replace deprecated imports from
org.springframework.boot.test.mock.mockito with the Spring Boot 3.4.9-compatible
ones: use org.springframework.test.context.bean.override.mockito.MockitoBean and
org.springframework.test.context.bean.override.mockito.MockitoSpyBean in
AbstractCacheTest (and any test classes using MockBean/SpyBean). Update
class-level annotations to @MockitoBean/@MockitoSpyBean where applicable and
remove redundant @Autowired annotations on those fields since the bean-override
annotations handle injection. Ensure any existing usages of MockBean/SpyBean are
converted consistently to the new annotations and run tests to confirm no
injection breakage.
In @backend/app/src/test/kotlin/io/tolgee/controllers/MarketingEmailingTest.kt:
- Around line 27-42: Replace usages of @MockBean with the Spring Boot 3.4+
recommended @MockitoBean
(org.springframework.test.context.bean.override.mockito.MockitoBean) for the
mocked fields in MarketingEmailingTest and remove the redundant @Autowired
annotations; specifically, change the annotations on the
mailjetEmailServiceManager property (currently annotated with @MockBean and
@Autowired) to a single @MockitoBean and on the restTemplate property (currently
@MockBean/@Autowired private val restTemplate: RestTemplate? = null) to a single
@MockitoBean, letting the framework inject the mocks and deleting the
unnecessary @Autowired imports/usages.
In @backend/testing/src/main/kotlin/io/tolgee/fixtures/EmailTestUtil.kt:
- Line 16: Replace the deprecated MockBean usage by importing and using
MockitoBean: change the import from
org.springframework.boot.test.mock.mockito.MockBean to
org.springframework.test.context.bean.override.mockito.MockitoBean and update
any @MockBean annotations in EmailTestUtil (and related classes) to
@MockitoBean, keeping the same attributes/targets so behavior stays identical.
In
@backend/testing/src/main/kotlin/io/tolgee/fixtures/MachineTranslationTest.kt:
- Around line 21-27: Replace the deprecated @MockBean usages: remove the
redundant @Autowired and @MockBean annotations on the googleTranslate and
amazonTranslate fields and annotate them with @MockitoBean (from
org.springframework.test.context.bean.override.mockito) instead; update imports
to use org.springframework.test.context.bean.override.mockito.MockitoBean and
ensure the field declarations for googleTranslate: Translate and
amazonTranslate: TranslateClient remain as-is so MockitoBean will register and
inject the mocks.
In
@ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/ContentDeliveryConfigControllerEeTest.kt:
- Line 33: In ContentDeliveryConfigControllerEeTest.kt revert the deprecated
@SpyBean import/usage to the Spring Boot 3.4+ recommended @MockitoSpyBean (use
org.springframework.test.context.bean.override.mockito.MockitoSpyBean) for any
spy fields, and remove redundant @Autowired on those spy-annotated fields since
the spy annotation performs injection automatically; update the import line
(remove org.springframework.boot.test.mock.mockito.SpyBean, add
org.springframework.test.context.bean.override.mockito.MockitoSpyBean) and
replace any @SpyBean annotations with @MockitoSpyBean on the corresponding test
fields.
In
@ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/ContentStorageControllerTest.kt:
- Line 30: The test incorrectly imports and uses the deprecated @SpyBean; revert
to the Spring 6.2+ recommended annotation by replacing the import
org.springframework.boot.test.mock.mockito.SpyBean with
org.springframework.boot.test.mock.mockito.MockitoSpyBean and update any
@SpyBean annotations in ContentStorageControllerTest (and adjacent tests in the
same file) to @MockitoSpyBean so the tests use the newer MockitoSpyBean
proxy/lifecycle behavior.
In
@ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/EeSubscriptionUsageControllerTest.kt:
- Around line 20-28: Update the test to use the Spring Boot 3.4-compatible
Mockito annotation: replace the import of
org.springframework.boot.test.mock.mockito.MockBean with
org.springframework.test.context.mockito.MockitoBean and change the annotation
on the restTemplate field from @MockBean to @MockitoBean (the field is the
lateinit var restTemplate: RestTemplate inside
EeSubscriptionUsageControllerTest).
In
@ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/slack/SlackLoginControllerTest.kt:
- Line 20: Replace deprecated Spring Boot @MockBean usage with @MockitoBean:
update the import from org.springframework.boot.test.mock.mockito.MockBean to
org.springframework.test.context.bean.override.mockito.MockitoBean and change
any annotation usages of @MockBean in SlackLoginControllerTest (and the other
occurrences referenced around lines 33-35) to @MockitoBean; ensure imports are
updated and tests compile under Spring Boot 3.4.9.
In
@ee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/CreditLimitTest.kt:
- Around line 27-28: Remove the unused imports
org.springframework.boot.test.mock.mockito.MockBean and
org.springframework.boot.test.mock.mockito.SpyBean from the top of the file; the
test uses @MockitoBean and @MockitoSpyBean (from
org.springframework.test.context.bean.override.mockito) so delete those two
unused import lines to clean up unused imports.
In
@ee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/KeyUsageReportingTest.kt:
- Line 19: In KeyUsageReportingTest.kt replace the deprecated MockBean usage:
change the import from org.springframework.boot.test.mock.mockito.MockBean to
org.springframework.test.context.bean.override.mockito.MockitoBean, update any
@MockBean annotations to @MockitoBean on the test fields, and remove the
redundant @Autowired on those fields (MockitoBean provides injection). Ensure
the annotation target and field types remain unchanged so the test still injects
the mocked beans.
In @ee/backend/tests/src/test/kotlin/io/tolgee/ee/slack/SlackIntegrationTest.kt:
- Around line 20-28: Replace the deprecated Spring Boot @MockBean usage with the
Spring Framework 6.2+ replacement @MockitoBean: in the SlackIntegrationTest
class, change the annotation on the slackClient field (lateinit var slackClient:
Slack) from @MockBean to @MockitoBean (import
org.springframework.test.context.bean.override.mockito.MockitoBean) and remove
the old @MockBean import to ensure compatibility with Spring Framework
6.2+/Spring Boot 3.4+.
🧹 Nitpick comments (11)
ee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/SeatUsageReportingTest.kt (1)
32-34: Redundant@Autowiredannotation with@MockBean.
@MockBeanalready handles injecting the mock into the field, so the@Autowiredannotation is unnecessary here. Consider removing it for cleaner code.Suggested fix
@MockBean - @Autowired lateinit var restTemplate: RestTemplatebackend/app/src/test/kotlin/io/tolgee/service/LanguageCachingTest.kt (1)
31-33: Redundant@Autowiredannotation.
@SpyBeanalready handles injection, so@Autowiredis unnecessary here.♻️ Suggested simplification
@SpyBean -@Autowired private lateinit var languageRepository: LanguageRepository
Note on deprecation:
@SpyBeanis deprecated in Spring Boot 3.4+ in favor of@MockitoSpyBeanfrom Spring Framework 6.2. If the project upgrades to Spring Boot 3.4+, this migration may need to be reversed.backend/app/src/test/kotlin/io/tolgee/automation/AutomationIntegrationTest.kt (1)
27-27: Migration from@MockitoBeanto@MockBeanlooks consistent.The change aligns with the PR-wide migration pattern. Note that
@MockBeanalready handles bean injection, so the@Autowiredannotation on lines 34 and 40 is redundant—though harmless.♻️ Optional: Remove redundant @Autowired
@MockBean -@Autowired lateinit var contentDeliveryFileStorageProvider: ContentDeliveryFileStorageProvider@MockBean -@Autowired lateinit var contentDeliveryCachePurgingProvider: ContentDeliveryCachePurgingProviderAlso applies to: 33-35, 39-41
ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/EeLicenseControllerTest.kt (1)
26-28:@Autowiredis redundant when combined with@MockBean.
@MockBeanalready creates the mock and registers it in the Spring context, handling injection automatically. The@Autowiredannotation can be safely removed.♻️ Suggested simplification
- @Autowired @MockBean lateinit var restTemplate: RestTemplatebackend/app/src/test/kotlin/io/tolgee/batch/BatchJobsGeneralWithRedisTest.kt (1)
20-20: Consider standardizing test spy beans on@MockitoSpyBeaninstead.This change uses
@SpyBeanfromspring-boot-test-mock, which is deprecated in Spring Boot 3.4+ (the current version in use). The newer@MockitoSpyBeanannotation fromorg.springframework.test.context.bean.override.mockitois the recommended approach for this Spring Boot version.While this aligns with the codebase's existing pattern (20+ test files use
@SpyBean), consider standardizing the project on@MockitoSpyBeanto align with Spring Boot 3.4+ best practices. This is not blocking but would improve long-term maintainability.backend/app/src/test/kotlin/io/tolgee/service/recaptchaValidation/RecaptchaValidationTest.kt (1)
38-40: Remove redundant@Autowiredwith@MockBean.The
@Autowiredannotation is redundant when using@MockBean, as the mock is automatically injected. Remove it for clarity.Additionally,
@MockBeanis deprecated in Spring Boot 3.4+ (project is on 3.4.9) in favor of@MockitoBeanfromorg.springframework.test.context.bean.override.mockito.MockitoBean. Consider migrating to the newer annotation.Suggested change (if staying with @MockBean)
- @Autowired @MockBean lateinit var restTemplate: RestTemplateee/backend/tests/src/test/kotlin/io/tolgee/ee/service/EeSubscriptionProviderImplTest.kt (1)
136-147: Consider simplifying the function signature.The extension function syntax
private fun EeSubscriptionProviderImplTest.prepareSubscription()is unusual for a method defined inside its own class. A regular private function would be clearer:Suggested simplification
- private fun EeSubscriptionProviderImplTest.prepareSubscription() { + private fun prepareSubscription() {backend/app/src/test/kotlin/io/tolgee/batch/AbstractBatchJobsGeneralTest.kt (1)
36-67: LGTM with optional cleanup suggestion.The
@SpyBeanannotation changes are consistent with the broader migration pattern across the project. The implementation is correct and will work as expected.Optional: The
@Autowiredannotation is redundant when combined with@SpyBean, as@SpyBeanalready handles bean registration and injection. You could simplify by removing@Autowiredfrom spied fields:@SpyBean lateinit var preTranslationByTmChunkProcessor: PreTranslationByTmChunkProcessorbackend/app/src/test/kotlin/io/tolgee/cache/AbstractCacheTest.kt (1)
40-64: Mock/Spy bean configuration is correct.The migration to
@MockBeanand@SpyBeanannotations is properly applied. The test mocking setup will function correctly.Minor note:
@Autowiredis redundant when combined with@MockBean/@SpyBean—these annotations already inject the mock into the field. Removing@Autowiredwould clean up the declarations, but it's not required.backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/BatchJobTestBase.kt (1)
36-47: Remove redundant@Autowiredannotations with@SpyBean.
@SpyBeanalready handles both creation and injection of the spy, making@Autowiredunnecessary on those same fields.Additionally,
@SpyBeanfromorg.springframework.boot.test.mock.mockitois deprecated in Spring Boot 3.4+ (your project uses 3.4.9). Migrate to@MockitoSpyBeanfromorg.springframework.test.context.bean.override.mockitowhen refactoring.♻️ Suggested cleanup
- @Autowired @SpyBean lateinit var machineTranslationProperties: MachineTranslationProperties @Autowired lateinit var entityManager: EntityManager var fakeBefore: Boolean = false - @Autowired @SpyBean private lateinit var internalProperties: InternalPropertiesee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/KeyCountLimitTest.kt (1)
106-106: Reminder: Address the TODO for subscription error status test.This TODO indicates a missing test case for the scenario when subscription status is ERROR. This would improve test coverage for edge cases.
Would you like me to generate a test case for this scenario, or open an issue to track this task?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (60)
backend/app/src/test/kotlin/io/tolgee/AuthProviderChangeTest.ktbackend/app/src/test/kotlin/io/tolgee/AuthTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/UserMfaControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/V2InvitationControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/V2UserControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/AbstractBatchJobManagementControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/BatchChangeTranslationStateTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/BatchClearTranslationsTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/BatchCopyTranslationsTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/BatchDeleteKeysTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/BatchJobTestBase.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/BatchMoveToNamespaceTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/BatchMtTranslateTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/BatchPreTranslateByTmTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/BatchTagKeysTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/contentDelivery/ContentDeliveryConfigControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/notification/NotificationSettingsControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/organizationController/OrganizationControllerInvitingTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translationSuggestionController/TranslationSuggestionControllerMtTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translationSuggestionController/TranslationSuggestionControllerStreamingTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translationSuggestionController/TranslationSuggestionWithCacheTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ImageUploadController/V2ImageUploadControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ProjectsController/ProjectsControllerInvitationTest.ktbackend/app/src/test/kotlin/io/tolgee/automation/AutomationCachingTest.ktbackend/app/src/test/kotlin/io/tolgee/automation/AutomationIntegrationTest.ktbackend/app/src/test/kotlin/io/tolgee/batch/AbstractBatchJobsGeneralTest.ktbackend/app/src/test/kotlin/io/tolgee/batch/BatchJobsGeneralWithRedisTest.ktbackend/app/src/test/kotlin/io/tolgee/cache/AbstractCacheTest.ktbackend/app/src/test/kotlin/io/tolgee/config/BatchJobBaseConfiguration.ktbackend/app/src/test/kotlin/io/tolgee/controllers/MarketingEmailingTest.ktbackend/app/src/test/kotlin/io/tolgee/controllers/resetPassword/ResetPasswordControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/security/EmailVerificationTest.ktbackend/app/src/test/kotlin/io/tolgee/service/LanguageCachingTest.ktbackend/app/src/test/kotlin/io/tolgee/service/TelemetryServiceTest.ktbackend/app/src/test/kotlin/io/tolgee/service/notification/NotificationServiceTest.ktbackend/app/src/test/kotlin/io/tolgee/service/organizationRole/OrganizationRoleCachingTest.ktbackend/app/src/test/kotlin/io/tolgee/service/recaptchaValidation/RecaptchaValidationTest.ktbackend/testing/src/main/kotlin/io/tolgee/config/TestEmailConfiguration.ktbackend/testing/src/main/kotlin/io/tolgee/fixtures/EmailTestUtil.ktbackend/testing/src/main/kotlin/io/tolgee/fixtures/MachineTranslationTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/EeTolgeeTranslatorSuggestingTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/SsoGlobalTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/SsoOrganizationsTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/WebhookAutomationTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/ContentDeliveryConfigControllerEeTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/ContentStorageControllerTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/EeLicenseControllerTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/EeSubscriptionUsageControllerTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/slack/OrganizationSlackControllerTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/slack/SlackLoginControllerTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/task/TaskControllerTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/CreditLimitTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/KeyCountLimitTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/KeyUsageReportingTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/ScheduledUsageReportingTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/SeatUsageReportingTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/service/EeSubscriptionProviderImplTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/slack/SlackIntegrationTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/slack/SlackWithAutoTranslationTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/slack/SlackWithBatchOperationTest.kt
💤 Files with no reviewable changes (14)
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/BatchChangeTranslationStateTest.kt
- backend/testing/src/main/kotlin/io/tolgee/config/TestEmailConfiguration.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/BatchMtTranslateTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/BatchClearTranslationsTest.kt
- backend/app/src/test/kotlin/io/tolgee/config/BatchJobBaseConfiguration.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/BatchDeleteKeysTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/BatchPreTranslateByTmTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/BatchTagKeysTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/V2InvitationControllerTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/notification/NotificationSettingsControllerTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/UserMfaControllerTest.kt
- backend/app/src/test/kotlin/io/tolgee/security/EmailVerificationTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/BatchCopyTranslationsTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/BatchMoveToNamespaceTest.kt
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/app/src/test/kotlin/io/tolgee/controllers/resetPassword/ResetPasswordControllerTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/organizationController/OrganizationControllerInvitingTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/V2UserControllerTest.kt
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: After modifying JPA entities, run./gradlew diffChangeLogto generate Liquibase changelog entries (add--no-daemonflag if docker command is not found)
Run specific test suites instead of the bare test task; use:data:test,:server-app:runStandardTests,:server-app:runContextRecreatingTests,:server-app:runWebsocketTests,:server-app:runWithoutEeTests,:ee-test:test, or:security:testwith the--testsflag for individual tests
Always run./gradlew ktlintFormatbefore commits in backend code
Files:
ee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/ScheduledUsageReportingTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/EeSubscriptionUsageControllerTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/KeyUsageReportingTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/slack/SlackWithAutoTranslationTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/EeLicenseControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/cache/AbstractCacheTest.ktbackend/app/src/test/kotlin/io/tolgee/AuthProviderChangeTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/ContentStorageControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/service/TelemetryServiceTest.ktbackend/app/src/test/kotlin/io/tolgee/service/LanguageCachingTest.ktbackend/app/src/test/kotlin/io/tolgee/batch/BatchJobsGeneralWithRedisTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/slack/SlackIntegrationTest.ktbackend/app/src/test/kotlin/io/tolgee/automation/AutomationCachingTest.ktbackend/app/src/test/kotlin/io/tolgee/service/recaptchaValidation/RecaptchaValidationTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translationSuggestionController/TranslationSuggestionWithCacheTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/slack/SlackLoginControllerTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/service/EeSubscriptionProviderImplTest.ktbackend/testing/src/main/kotlin/io/tolgee/fixtures/MachineTranslationTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ProjectsController/ProjectsControllerInvitationTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/AbstractBatchJobManagementControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ImageUploadController/V2ImageUploadControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/AuthTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/SsoOrganizationsTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/task/TaskControllerTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/EeTolgeeTranslatorSuggestingTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translationSuggestionController/TranslationSuggestionControllerStreamingTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/slack/SlackWithBatchOperationTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/SsoGlobalTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/SeatUsageReportingTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/KeyCountLimitTest.ktbackend/app/src/test/kotlin/io/tolgee/controllers/MarketingEmailingTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/CreditLimitTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/WebhookAutomationTest.ktbackend/app/src/test/kotlin/io/tolgee/automation/AutomationIntegrationTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/slack/OrganizationSlackControllerTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/ContentDeliveryConfigControllerEeTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translationSuggestionController/TranslationSuggestionControllerMtTest.ktbackend/app/src/test/kotlin/io/tolgee/batch/AbstractBatchJobsGeneralTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/BatchJobTestBase.ktbackend/app/src/test/kotlin/io/tolgee/service/notification/NotificationServiceTest.ktbackend/app/src/test/kotlin/io/tolgee/service/organizationRole/OrganizationRoleCachingTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/contentDelivery/ContentDeliveryConfigControllerTest.ktbackend/testing/src/main/kotlin/io/tolgee/fixtures/EmailTestUtil.kt
**/*.{ts,tsx,js,jsx,kt,kts}
⚙️ CodeRabbit configuration file
As part of review, please check if the file follows 'The Stepdown Rule': The most important, high-level concepts should be at the top (like a newspaper headline and opening paragraph). Details should increase as you read downward. Functions should be ordered so that a caller appears before the functions it calls. When working with JavaScript components, we allow the main component to live at the bottom of the file as an exception to the rule.
Files:
ee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/ScheduledUsageReportingTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/EeSubscriptionUsageControllerTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/KeyUsageReportingTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/slack/SlackWithAutoTranslationTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/EeLicenseControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/cache/AbstractCacheTest.ktbackend/app/src/test/kotlin/io/tolgee/AuthProviderChangeTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/ContentStorageControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/service/TelemetryServiceTest.ktbackend/app/src/test/kotlin/io/tolgee/service/LanguageCachingTest.ktbackend/app/src/test/kotlin/io/tolgee/batch/BatchJobsGeneralWithRedisTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/slack/SlackIntegrationTest.ktbackend/app/src/test/kotlin/io/tolgee/automation/AutomationCachingTest.ktbackend/app/src/test/kotlin/io/tolgee/service/recaptchaValidation/RecaptchaValidationTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translationSuggestionController/TranslationSuggestionWithCacheTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/slack/SlackLoginControllerTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/service/EeSubscriptionProviderImplTest.ktbackend/testing/src/main/kotlin/io/tolgee/fixtures/MachineTranslationTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ProjectsController/ProjectsControllerInvitationTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/AbstractBatchJobManagementControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ImageUploadController/V2ImageUploadControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/AuthTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/SsoOrganizationsTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/task/TaskControllerTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/EeTolgeeTranslatorSuggestingTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translationSuggestionController/TranslationSuggestionControllerStreamingTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/slack/SlackWithBatchOperationTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/SsoGlobalTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/SeatUsageReportingTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/KeyCountLimitTest.ktbackend/app/src/test/kotlin/io/tolgee/controllers/MarketingEmailingTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/CreditLimitTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/WebhookAutomationTest.ktbackend/app/src/test/kotlin/io/tolgee/automation/AutomationIntegrationTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/slack/OrganizationSlackControllerTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/ContentDeliveryConfigControllerEeTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translationSuggestionController/TranslationSuggestionControllerMtTest.ktbackend/app/src/test/kotlin/io/tolgee/batch/AbstractBatchJobsGeneralTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/BatchJobTestBase.ktbackend/app/src/test/kotlin/io/tolgee/service/notification/NotificationServiceTest.ktbackend/app/src/test/kotlin/io/tolgee/service/organizationRole/OrganizationRoleCachingTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/contentDelivery/ContentDeliveryConfigControllerTest.ktbackend/testing/src/main/kotlin/io/tolgee/fixtures/EmailTestUtil.kt
🧠 Learnings (6)
📓 Common learnings
Learnt from: cyyynthia
Repo: tolgee/tolgee-platform PR: 1987
File: email/components/translate.ts:36-37
Timestamp: 2025-06-16T20:31:22.217Z
Learning: In email template processing systems like React Email with Thymeleaf integration, build-time processes that run with NODE_ENV === 'production' are short-lived and terminate after completion, so memory accumulation in data structures like Sets is not a concern as the memory is automatically freed when the process ends.
📚 Learning: 2026-01-09T10:00:46.985Z
Learnt from: CR
Repo: tolgee/tolgee-platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T10:00:46.985Z
Learning: Applies to **/backend/**/*.test.kt : Use TestData classes for test setup in backend tests, following the pattern: create a TestData class, inject TestDataService, initialize in BeforeEach, and cleanup in AfterEach
Applied to files:
ee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/ScheduledUsageReportingTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/EeSubscriptionUsageControllerTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/KeyUsageReportingTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/EeLicenseControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/cache/AbstractCacheTest.ktbackend/app/src/test/kotlin/io/tolgee/AuthProviderChangeTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/ContentStorageControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/service/TelemetryServiceTest.ktbackend/app/src/test/kotlin/io/tolgee/service/LanguageCachingTest.ktbackend/app/src/test/kotlin/io/tolgee/batch/BatchJobsGeneralWithRedisTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/slack/SlackIntegrationTest.ktbackend/app/src/test/kotlin/io/tolgee/automation/AutomationCachingTest.ktbackend/app/src/test/kotlin/io/tolgee/service/recaptchaValidation/RecaptchaValidationTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translationSuggestionController/TranslationSuggestionWithCacheTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/slack/SlackLoginControllerTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/service/EeSubscriptionProviderImplTest.ktbackend/testing/src/main/kotlin/io/tolgee/fixtures/MachineTranslationTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ProjectsController/ProjectsControllerInvitationTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/AbstractBatchJobManagementControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ImageUploadController/V2ImageUploadControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/AuthTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/SsoOrganizationsTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/task/TaskControllerTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/EeTolgeeTranslatorSuggestingTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translationSuggestionController/TranslationSuggestionControllerStreamingTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/SsoGlobalTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/SeatUsageReportingTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/KeyCountLimitTest.ktbackend/app/src/test/kotlin/io/tolgee/controllers/MarketingEmailingTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/CreditLimitTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/WebhookAutomationTest.ktbackend/app/src/test/kotlin/io/tolgee/automation/AutomationIntegrationTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/slack/OrganizationSlackControllerTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/ContentDeliveryConfigControllerEeTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translationSuggestionController/TranslationSuggestionControllerMtTest.ktbackend/app/src/test/kotlin/io/tolgee/batch/AbstractBatchJobsGeneralTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/BatchJobTestBase.ktbackend/app/src/test/kotlin/io/tolgee/service/notification/NotificationServiceTest.ktbackend/app/src/test/kotlin/io/tolgee/service/organizationRole/OrganizationRoleCachingTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/contentDelivery/ContentDeliveryConfigControllerTest.ktbackend/testing/src/main/kotlin/io/tolgee/fixtures/EmailTestUtil.kt
📚 Learning: 2025-06-11T16:17:35.696Z
Learnt from: cyyynthia
Repo: tolgee/tolgee-platform PR: 3126
File: backend/testing/build.gradle:78-78
Timestamp: 2025-06-11T16:17:35.696Z
Learning: The Gradle module `backend/testing` serves as an internal test pseudo-library; therefore, test-scope dependencies such as `org.springframework.boot:spring-boot-starter-test` are intentionally declared with `implementation`, as the module is only consumed on the test classpath and is not part of production artifacts.
Applied to files:
backend/app/src/test/kotlin/io/tolgee/cache/AbstractCacheTest.ktbackend/app/src/test/kotlin/io/tolgee/service/TelemetryServiceTest.ktbackend/app/src/test/kotlin/io/tolgee/service/recaptchaValidation/RecaptchaValidationTest.ktbackend/app/src/test/kotlin/io/tolgee/automation/AutomationIntegrationTest.ktbackend/app/src/test/kotlin/io/tolgee/service/organizationRole/OrganizationRoleCachingTest.kt
📚 Learning: 2025-10-07T14:36:39.331Z
Learnt from: Anty0
Repo: tolgee/tolgee-platform PR: 3263
File: backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationRoleService.kt:279-290
Timestamp: 2025-10-07T14:36:39.331Z
Learning: In OrganizationRoleService in backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationRoleService.kt, the `removeOrDeactivateUser` method intentionally has two distinct paths: if a user can be removed, it calls `removeUserForReal` (which removes organization roles, project permissions, and evicts caches); if a user is managed by the organization and cannot be removed, it calls `userAccountService.disable(userId)` without any cleanup. This design is intentional—disabling should be reversible so the user stays unmodified (keeping their organization roles and project permissions intact), allowing them to be re-enabled if they return to the organization.
Applied to files:
backend/app/src/test/kotlin/io/tolgee/cache/AbstractCacheTest.ktbackend/app/src/test/kotlin/io/tolgee/service/organizationRole/OrganizationRoleCachingTest.kt
📚 Learning: 2026-01-09T10:00:46.985Z
Learnt from: CR
Repo: tolgee/tolgee-platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T10:00:46.985Z
Learning: Applies to **/backend/**/*.test.kt : Use `.andAssertThatJson` for asserting API response JSON in backend tests instead of manual assertions
Applied to files:
ee/backend/tests/src/test/kotlin/io/tolgee/ee/service/EeSubscriptionProviderImplTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ProjectsController/ProjectsControllerInvitationTest.ktbackend/app/src/test/kotlin/io/tolgee/AuthTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/SsoOrganizationsTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/SsoGlobalTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/WebhookAutomationTest.kt
📚 Learning: 2026-01-09T10:00:46.985Z
Learnt from: CR
Repo: tolgee/tolgee-platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T10:00:46.985Z
Learning: Create E2E test data with 3 components: TestData class in `backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/`, E2E Data Controller in `backend/development/src/main/kotlin/io/tolgee/controllers/internal/e2eData/`, and frontend test data object in `e2e/cypress/common/apiCalls/testData/testData.ts`
Applied to files:
backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ProjectsController/ProjectsControllerInvitationTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/task/TaskControllerTest.ktee/backend/tests/src/test/kotlin/io/tolgee/ee/EeTolgeeTranslatorSuggestingTest.kt
⏰ 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). (3)
- GitHub Check: Frontend static check 🪲
- GitHub Check: Build backend 🏗️
- GitHub Check: Build frontend 🏗️
🔇 Additional comments (25)
ee/backend/tests/src/test/kotlin/io/tolgee/ee/WebhookAutomationTest.kt (1)
37-44: LGTM! Annotation migration looks correct.The migration from
@MockitoBeanto Spring Boot's@MockBeanis appropriate for standardizing test infrastructure. Both annotations functionally create and inject mock beans into the Spring context.Minor note: The
@Autowiredannotation on these fields is redundant since@MockBeanalready handles both bean registration and injection. Removing it would be a minor cleanup, but it's harmless to keep.ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/task/TaskControllerTest.kt (1)
109-114: LGTM! Async email assertion pattern correctly applied.The
waitForNotThrowingwrapper properly handles the eventual consistency of async email delivery.backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ProjectsController/ProjectsControllerInvitationTest.kt (3)
18-18: LGTM!Import added for the async polling utility used in email verification tests.
179-189: LGTM!The async polling pattern correctly handles the asynchronous email delivery. The verification waits for the email to be sent before accessing the captured message content for assertions.
193-201: LGTM!Consistent application of the async polling pattern for email verification.
backend/app/src/test/kotlin/io/tolgee/service/notification/NotificationServiceTest.kt (2)
5-5: LGTM!Import addition aligns with the async email testing pattern being adopted across the test suite.
84-92: LGTM!The
waitForNotThrowingwrapper correctly handles the now-asynchronous email delivery. The timeout and poll interval are reasonable.Note: The
assertEmailNotificationNotExists()helper doesn't use polling, which is acceptable since when the channel is disabled, no email should be queued in the first place. If flakiness is observed in those negative tests, consider adding a brief delay before asserting absence.backend/app/src/test/kotlin/io/tolgee/AuthTest.kt (1)
23-23: Incorrect migration direction: This is moving toward the deprecated annotation.Spring Boot 3.4+ deprecated
@MockBeanin favor of@MockitoBeanfrom Spring Framework 6.2. The import and annotation should be changed toorg.springframework.test.context.bean.override.mockito.MockitoBeaninstead.Replace:
import org.springframework.boot.test.mock.mockito.MockBeanwithimport org.springframework.test.context.bean.override.mockito.MockitoBean@MockBeanannotation with@MockitoBeanLikely an incorrect or invalid review comment.
backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translationSuggestionController/TranslationSuggestionControllerMtTest.kt (1)
45-45: No action needed. This change was an intentional revert.The git history shows commit
01690009explicitly reverts PR #3366 ("test: cleanup deprecated mockito annotations"), which had converted to@MockitoBean. The use of@MockBeanin this file is therefore intentional, not an oversight or regression. While the project runs Spring Boot 3.4.9 (which introduced@MockitoBean), the deliberate revert suggests the team encountered issues with the newer annotation and opted to maintain@MockBeanacross the test suite for stability.Likely an incorrect or invalid review comment.
backend/app/src/test/kotlin/io/tolgee/controllers/MarketingEmailingTest.kt (1)
64-69: Setup correctly clears mock invocations.The
@BeforeEachproperly clears invocations on the mock and initializes test utilities. This ensures test isolation when using the@MockBean-injected mock.backend/app/src/test/kotlin/io/tolgee/service/organizationRole/OrganizationRoleCachingTest.kt (1)
28-28: Migrate back to@MockitoSpyBean— this change uses deprecated annotations.In Spring Boot 3.4+,
@SpyBean(fromorg.springframework.boot.test.mock.mockito) is deprecated. The project targets Spring Boot 3.4.9, so this change should use@MockitoSpyBean(fromorg.springframework.test.context.bean.override.mockito) instead. Update the import on line 28 and the annotations on lines 37 and 43.⛔ Skipped due to learnings
Learnt from: Anty0 Repo: tolgee/tolgee-platform PR: 3263 File: backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationRoleService.kt:279-290 Timestamp: 2025-10-07T14:36:39.331Z Learning: In OrganizationRoleService in backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationRoleService.kt, the `removeOrDeactivateUser` method intentionally has two distinct paths: if a user can be removed, it calls `removeUserForReal` (which removes organization roles, project permissions, and evicts caches); if a user is managed by the organization and cannot be removed, it calls `userAccountService.disable(userId)` without any cleanup. This design is intentional—disabling should be reversible so the user stays unmodified (keeping their organization roles and project permissions intact), allowing them to be re-enabled if they return to the organization.Learnt from: CR Repo: tolgee/tolgee-platform PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-01-09T10:00:46.985Z Learning: Applies to **/backend/**/*.test.kt : Use TestData classes for test setup in backend tests, following the pattern: create a TestData class, inject TestDataService, initialize in BeforeEach, and cleanup in AfterEachLearnt from: cyyynthia Repo: tolgee/tolgee-platform PR: 3126 File: backend/testing/build.gradle:78-78 Timestamp: 2025-06-11T16:17:35.696Z Learning: The Gradle module `backend/testing` serves as an internal test pseudo-library; therefore, test-scope dependencies such as `org.springframework.boot:spring-boot-starter-test` are intentionally declared with `implementation`, as the module is only consumed on the test classpath and is not part of production artifacts.backend/testing/src/main/kotlin/io/tolgee/fixtures/EmailTestUtil.kt (1)
32-41: LGTM! Initialization ofbackEndUrlfor email template testing.The addition of
tolgeeProperties.backEndUrl = "https://example.com"correctly configures a mock backend URL needed by the new email template system that generates links. This ensures tests have a predictable URL for assertions.ee/backend/tests/src/test/kotlin/io/tolgee/ee/service/EeSubscriptionProviderImplTest.kt (1)
20-20: Replace deprecated@MockBeanwith@MockitoBeanfor Spring Boot 3.4.9 compatibility.The project uses Spring Boot 3.4.9, where
@MockBeanfromorg.springframework.boot.test.mock.mockitois deprecated since 3.4.0. Replace it with@MockitoBeanfromorg.springframework.test.context.bean.override.mockito.Additionally, the
@Autowiredannotation on line 36 is redundant—@MockitoBeanautomatically handles injection into the test class field.⛔ Skipped due to learnings
Learnt from: CR Repo: tolgee/tolgee-platform PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-01-09T10:00:46.985Z Learning: Applies to **/backend/**/*.test.kt : Use TestData classes for test setup in backend tests, following the pattern: create a TestData class, inject TestDataService, initialize in BeforeEach, and cleanup in AfterEachLearnt from: cyyynthia Repo: tolgee/tolgee-platform PR: 3126 File: backend/testing/build.gradle:78-78 Timestamp: 2025-06-11T16:17:35.696Z Learning: The Gradle module `backend/testing` serves as an internal test pseudo-library; therefore, test-scope dependencies such as `org.springframework.boot:spring-boot-starter-test` are intentionally declared with `implementation`, as the module is only consumed on the test classpath and is not part of production artifacts.ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/slack/OrganizationSlackControllerTest.kt (1)
24-24: LGTM!The
@MockBeanmigration is consistent with the pattern applied inSlackLoginControllerTest.ktand other files in this PR. The@MockBean+@Autowiredcombination onrestTemplateis a valid Spring test pattern for injecting a mock into the test class while also replacing the bean in the application context.Also applies to: 39-41
ee/backend/tests/src/test/kotlin/io/tolgee/ee/selfHostedLimitsAndReporting/ScheduledUsageReportingTest.kt (1)
44-150: LGTM on the test logic and structure.The test properly:
- Cancels scheduled tasks in
@BeforeAllfor isolation- Resets spies in
@AfterEach- Uses
waitForNotThrowingwith appropriate timeouts for async assertions- The
Thread.sleepusage for negative assertions (verifying something doesn't happen prematurely) is an acceptable pattern herebackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translationSuggestionController/TranslationSuggestionWithCacheTest.kt (1)
68-72: Stubbing change aligns with mock-based approach.The change from direct property assignment to
whenever(...).thenAnsweris necessary whenmachineTranslationPropertiesis a mock or spy, as mocks don't hold state in their fields. This is consistent with the broader migration to Spring Boot test mocks.backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ImageUploadController/V2ImageUploadControllerTest.kt (1)
24-24: LGTM!The migration from
@MockitoBeanto@MockBeanis straightforward. The existingwheneverstubbing in the test methods remains compatible with the Spring Boot test mock.Also applies to: 44-46
backend/app/src/test/kotlin/io/tolgee/service/TelemetryServiceTest.kt (1)
21-21: LGTM!The annotation migration is correct. The
Mockito.reset(restTemplate)call andmockHttpRequesthelper work consistently with both@MockitoBeanand@MockBean.Also applies to: 32-34
backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/batch/AbstractBatchJobManagementControllerTest.kt (1)
20-20: LGTM!The migration from
@MockitoSpyBeanto@SpyBeanis consistent across all five spied dependencies. TheMockito.reset()call inabstractSetup()properly clears invocations between tests, ensuring test isolation.Also applies to: 42-43, 46-47, 50-51, 54-55, 57-59
backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translationSuggestionController/TranslationSuggestionControllerStreamingTest.kt (1)
23-24: LGTM!The migration correctly uses
@SpyBeanformtService(allowing partial mocking with real method behavior) and@MockBeanforeeSubscriptionInfoProvider(full mock). The existing stubbing viawhenever()andMockito.clearInvocations()remain compatible.Also applies to: 28-30, 37-39
backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/contentDelivery/ContentDeliveryConfigControllerTest.kt (1)
27-42: Same annotation migration pattern as other test files.The
@MockitoSpyBean→@SpyBeanmigration follows the same pattern seen in other test files. As with@MockBean,@SpyBeanwas deprecated in Spring Boot 3.4 in favor of@MockitoSpyBean.The test structure and logic are sound, with proper setup/teardown and spy reset handling.
ee/backend/tests/src/test/kotlin/io/tolgee/ee/SsoGlobalTest.kt (1)
31-51: Consistent mock annotation migration.The
@MockitoBean→@MockBeanchange is consistent with the migration pattern across other test files in this PR. The test class follows proper practices with TestData setup/cleanup and comprehensive SSO scenario coverage.ee/backend/tests/src/test/kotlin/io/tolgee/ee/EeTolgeeTranslatorSuggestingTest.kt (1)
22-30: Consistent mock annotation migration.The
@MockitoBean→@MockBeanchange follows the same pattern as other test files. The test properly uses TestData class (SuggestionTestData) and configures the mock within the test method.ee/backend/tests/src/test/kotlin/io/tolgee/ee/SsoOrganizationsTest.kt (1)
33-53: Consistent mock annotation migration.The
@MockitoBean→@MockBeanchange is consistent with other test files in this PR. The test class properly follows the TestData pattern withSsoTestData, including proper lifecycle management in@BeforeEachand@AfterEachmethods.ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/ContentDeliveryConfigControllerEeTest.kt (1)
57-71: LGTM!The test setup follows the recommended TestData pattern: initializing
testDatain@BeforeEach, saving viatestDataService.saveTestData(), and properly resetting mocks. TheMockito.reset()calls on the spy beans ensure clean state between tests.
0169000 to
6e0c9a1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/actions/setup-env/action.yaml (1)
14-26: Consider documenting credential requirements for Docker Hub login.The
docker-hub-usernameanddocker-hub-passwordinputs are required whendocker-hubis"true", but this dependency isn't explicitly documented. While the login step will fail if credentials are missing, an explicit note in the description would improve usability.📝 Suggested documentation improvement
docker-hub: - description: Whether to login to Docker Hub or not. Defaults to false. + description: Whether to login to Docker Hub or not. Requires docker-hub-username and docker-hub-password. Defaults to false. default: "false" docker-hub-username: - description: Username to use to authenticate to Docker Hub. + description: Username to use to authenticate to Docker Hub. Required if docker-hub is true. docker-hub-password: - description: Password to use to authenticate to Docker Hub. + description: Password to use to authenticate to Docker Hub. Required if docker-hub is true.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/actions/setup-env/action.yaml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: cyyynthia
Repo: tolgee/tolgee-platform PR: 1987
File: email/components/translate.ts:36-37
Timestamp: 2025-06-16T20:31:22.217Z
Learning: In email template processing systems like React Email with Thymeleaf integration, build-time processes that run with NODE_ENV === 'production' are short-lived and terminate after completion, so memory accumulation in data structures like Sets is not a concern as the memory is automatically freed when the process ends.
⏰ 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). (3)
- GitHub Check: Build frontend 🏗️
- GitHub Check: Build backend 🏗️
- GitHub Check: Frontend static check 🪲
🔇 Additional comments (3)
.github/actions/setup-env/action.yaml (3)
57-63: LGTM!The QEMU and Buildx setup steps are correctly guarded by the
dockerinput flag and use the appropriate official Docker actions at v3.
65-70: LGTM!The Docker Hub login step correctly uses input parameters for credentials and is properly guarded by the
docker-hubflag.
72-78: LGTM!The GHCR login step correctly uses the built-in
github.actorandgithub.tokencontext variables, which is the recommended approach for authenticating to GitHub Container Registry.
f7f9495 to
c33b437
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release.yml (1)
59-78:⚠️ Potential issue | 🔴 CriticalRemove the duplicate second "Create docker image" step (lines 72–78) — it redundantly rebuilds and pushes the same images.
The first step (lines 59–70) uses
./gradlew dockerPublishtwice to build and pushtolgee/tolgee:VERSIONandtolgee/tolgee:latesttags via Gradle, which automatically runsdockerPrepareto set up thebuild/dockerdirectory. The second step (lines 72–78) does the same work again usingdocker buildx builddirectly frombuild/docker, creating:
- Duplicate builder instances (
docker buildx create --usecalled twice)- Identical images pushed to the same registry
- Double CI build time
- Undocumented reliance on
build/dockerexisting (without explicit Gradle setup)Keep the first step, which is properly integrated with the build system.
🤖 Fix all issues with AI agents
In @.github/workflows/test.yml:
- Line 425: The bash script behind the "everything-passed" step currently checks
results for several jobs but omits the "email-code-checks" job, so failures
there are ignored; update the script to add the same result-check block used for
other jobs to verify the status of "email-code-checks" (e.g., read the job
result from the GitHub API/outputs and treat non-"success" as failure), mirror
the existing logic and exit non-zero or set the overall flag when
"email-code-checks" result is not success so the workflow correctly reports
failure.
In
`@backend/api/src/main/kotlin/io/tolgee/controllers/resetPassword/ResetPasswordRequestHandler.kt`:
- Around line 49-50: The email header currently always reads "Password reset"
while subject already varies by isInitial; update the header assignment in
ResetPasswordRequestHandler to mirror the subject logic so when isInitial is
true the header becomes "Initial password configuration" and otherwise remains
"Password reset" (i.e. change header = "Password reset" to a conditional header
= if (isInitial) "Initial password configuration" else "Password reset"
alongside the existing subject logic).
In `@backend/app/build.gradle`:
- Around line 282-289: There are two duplicate configurations for the bootJar
task; remove the older plain bootJar { ... } block and keep the new
tasks.named('bootJar', Jar) { ... } configuration (which sets
duplicatesStrategy, archiveFileName and manifest attributes) to avoid
duplicate/conflicting task definitions; update or remove the earlier bootJar
block so only the tasks.named('bootJar', Jar) configuration remains.
In `@backend/data/src/main/kotlin/io/tolgee/email/EmailMessageResolver.kt`:
- Around line 50-78: The code in EmailMessageResolver that manipulates the local
'stack' (constructed from code.replace(".", "--")) can throw
NoSuchElementException when encountering unmatched closing tags at
stack.removeLast(); update the logic to guard and provide a clearer error:
before calling stack.removeLast() in the closing-tag branch of the m.replaceAll
lambda, check whether the stack is non-empty (or whether the last element
matches the expected tag) and if not throw or rethrow a descriptive exception
that includes the problematic i18n message and tag (or catch
NoSuchElementException around the replaceAll and throw a new
IllegalStateException/IllegalArgumentException with that context); this will
pinpoint malformed i18n strings instead of surfacing a generic removeLast
failure.
In `@backend/data/src/main/kotlin/io/tolgee/email/EmailService.kt`:
- Around line 40-46: The smtpFrom getter in EmailService currently throws
IllegalStateException at runtime inside `@Async` methods (via
tolgeeProperties.smtp.from), which will be swallowed by the async executor; to
fix, validate SMTP configuration on startup by adding a startup check (e.g., in
EmailService's `@PostConstruct` method) that asserts tolgeeProperties.smtp.from is
present and throws a clear startup exception if missing, or alternatively
configure the async executor with an AsyncUncaughtExceptionHandler to surface
exceptions from async methods—update EmailService (smtpFrom), add a
`@PostConstruct` validation, or ensure the configured
AsyncUncaughtExceptionHandler is registered so missing SMTP config is reported
immediately.
- Around line 100-107: The title-extraction regex in extractEmailTitle uses
Regex("<title>(.+?)</title>") which fails on multi-line title content; update
the companion object REGEX_TITLE to compile with RegexOption.DOT_MATCHES_ALL
(e.g., Regex("<title>(.+?)</title>", RegexOption.DOT_MATCHES_ALL)) so the dot
also matches newlines and extractEmailTitle will correctly find titles spanning
multiple lines.
🧹 Nitpick comments (4)
backend/data/src/main/kotlin/io/tolgee/configuration/HibernateConfig.kt (1)
1-2: Nit: Copyright year says 2024, but this is a new file created in 2026.Suggested fix
-/** - * Copyright (C) 2024 Tolgee s.r.o. and contributors +/** + * Copyright (C) 2026 Tolgee s.r.o. and contributors.github/actions/upload-backend-build/action.yaml (1)
17-27: Unquoted$SRC_DIRin tar commands could break on paths with spaces.All tar commands use
$SRC_DIRunquoted. Ifsource-directoryever contains whitespace or glob characters, word-splitting will break the command. CI paths are typically safe, but quoting is a good defensive habit.Proposed fix: quote variable expansions
- tar --zstd -cf ~/backend-api.tar.zst $SRC_DIR/backend/api/build - tar --zstd -cf ~/backend-app.tar.zst $SRC_DIR/backend/app/build - tar --zstd -cf ~/backend-data.tar.zst $SRC_DIR/backend/data/build - tar --zstd -cf ~/backend-misc.tar.zst $SRC_DIR/backend/misc/build - tar --zstd -cf ~/backend-security.tar.zst $SRC_DIR/backend/security/build - tar --zstd -cf ~/backend-testing.tar.zst $SRC_DIR/backend/testing/build - tar --zstd -cf ~/backend-ktlint.tar.zst $SRC_DIR/backend/ktlint/build - tar --zstd -cf ~/backend-development.tar.zst $SRC_DIR/backend/development/build - tar --zstd -cf ~/ee-backend-app.tar.zst $SRC_DIR/ee/backend/app/build - tar --zstd -cf ~/ee-backend-tests.tar.zst $SRC_DIR/ee/backend/tests/build - tar --zstd -cf ~/backend-email.tar.zst $SRC_DIR/email/out + tar --zstd -cf ~/backend-api.tar.zst "$SRC_DIR/backend/api/build" + tar --zstd -cf ~/backend-app.tar.zst "$SRC_DIR/backend/app/build" + tar --zstd -cf ~/backend-data.tar.zst "$SRC_DIR/backend/data/build" + tar --zstd -cf ~/backend-misc.tar.zst "$SRC_DIR/backend/misc/build" + tar --zstd -cf ~/backend-security.tar.zst "$SRC_DIR/backend/security/build" + tar --zstd -cf ~/backend-testing.tar.zst "$SRC_DIR/backend/testing/build" + tar --zstd -cf ~/backend-ktlint.tar.zst "$SRC_DIR/backend/ktlint/build" + tar --zstd -cf ~/backend-development.tar.zst "$SRC_DIR/backend/development/build" + tar --zstd -cf ~/ee-backend-app.tar.zst "$SRC_DIR/ee/backend/app/build" + tar --zstd -cf ~/ee-backend-tests.tar.zst "$SRC_DIR/ee/backend/tests/build" + tar --zstd -cf ~/backend-email.tar.zst "$SRC_DIR/email/out"backend/data/src/main/kotlin/io/tolgee/component/email/TolgeeEmailSender.kt (1)
12-16: ConsiderbuildMapfor cleaner property construction.The chained
.let { ... it.plus(...) }pattern works but is a bit verbose and allocates intermediate maps.buildMapis more idiomatic and avoids the!!assertions.♻️ Proposed refactor
- val properties = - mapOf<String, Any>() - .let { if (params.text != null) it.plus("content" to params.text!!) else it } - .let { if (params.header != null) it.plus("header" to params.header!!) else it } - .let { if (params.recipientName != null) it.plus("recipientName" to params.recipientName!!) else it } + val properties = buildMap<String, Any> { + params.text?.let { put("content", it) } + params.header?.let { put("header", it) } + params.recipientName?.let { put("recipientName", it) } + }backend/data/src/main/kotlin/io/tolgee/email/EmailService.kt (1)
82-84: Use the standard "UTF-8" charset name instead of "UTF8".While Java accepts "UTF8" as a historical alias, "UTF-8" is the standard IANA charset name and is more widely recognized.
♻️ Proposed fix
- val helper = MimeMessageHelper(message, MimeMessageHelper.MULTIPART_MODE_MIXED_RELATED, "UTF8") + val helper = MimeMessageHelper(message, MimeMessageHelper.MULTIPART_MODE_MIXED_RELATED, "UTF-8")
dcdb77f to
6c19372
Compare
0f22178 to
ccaef55
Compare
- Add missing `header` field to InvitationEmailSender so invitation emails display a header, consistent with other email senders - Remove misleading `@Async` from EmailService.sendEmail; the method is always called from the already-@async sendEmailTemplate internally, so the annotation had no effect and implied false double-async behaviour - Add kaptGenerateStubsKotlin to the SKIP_SERVER_BUILD task exclusion list in build.gradle, restoring parity with the previous implementation - Unify URL extraction regexes in e2e common.ts: getParsedEmailVerification, getParsedEmailVerificationByIndex (index===0 path), and getParsedResetPasswordEmail now all use the href-attribute approach, which correctly handles https and a broader set of URL characters - Add KDoc to EmailParams.text clarifying the field holds HTML content and callers are responsible for escaping user-supplied values - Add comment in default.tsx explaining why dangerouslyInjectValueAsHtmlWithoutSanitization is intentional and what constraints callers must satisfy Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Move delete() and mkdir() to doLast in Gradle tasks to avoid configuration-time side effects (cleanDocker, createBuildDir) - Fix subprojects.forEach to iterate each subproject's tasks instead of repeatedly configuring server-app's tasks (diffChangelog) - Fix tolgeerc.json paths to be relative to config file location - Add missing email-code-checks result check in everything-passed CI gate Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove duplicate docker buildx step and permissions change that were accidentally left after a partial rollback. Restores release.yml to match main. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
bbb6530 to
bd4d491
Compare
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This PR introduces tooling for writing HTML emails using React Email, that is then compiled to Thymeleaf templates the backend can consume.
Features
The email design isn't the one made in #2710, but rather the first draft I originally made when first prototyping with this more than a year ago. (quick note: the design has a different footer based on whether it's sent from Cloud or Self-Hosted). This is why I'm marking this as "chore" more than "feat"; it doesn't bring any feature to the product yet.
I added
exportsto the package.json as a potential way to have additional email template workspaces ineeandbilling: those would be able to import core parts like the layout and essential components/parts, while also defining their own component/parts separately, providing good isolation of them and keeping clear legal boundaries between Tolgee OSS, Tolgee EE, and proprietary Tolgee Cloud licenses.I included documentation about how to use it, in
email/HACKING.md. Gradle config has been updated to build emails as part of the backend build process.Closes #2707
Summary by CodeRabbit
New Features
Chores