Skip to content

Comments

Feat/2286: Dynamically handle get started button in the product site based on session#737

Open
hasalarootcode wants to merge 2 commits intomainfrom
feat/2286-dymamically-handle-get-started-button-in-the-product-site-based-on-session
Open

Feat/2286: Dynamically handle get started button in the product site based on session#737
hasalarootcode wants to merge 2 commits intomainfrom
feat/2286-dymamically-handle-get-started-button-in-the-product-site-based-on-session

Conversation

@hasalarootcode
Copy link
Contributor

@hasalarootcode hasalarootcode commented Jul 10, 2025

PR checklist

TaskId: (https://github.com/rootcodelabs/skapp-ep/issues/2286)

Summary

How to test

Project Checklist

  • Changes build without any errors
  • Have written adequate test cases
  • Done developer testing in
    • Chrome
    • Firefox
    • Safari
  • Code is formatted with npm run format
  • Code is linted with npm run check-lint
  • No unnecessary comments left in code
  • Made corresponding changes to the documentation

Other

  • New atomic components added
  • New molecules added
  • New pages(routes) added
  • New dependencies installed

PR Checklist

  • Pull request is raised from the correct source branch
  • Pull request is raised to the correct destination branch
  • Pull request is raised with correct title
  • Pull request is self reviewed
  • Pull request is self assigned
  • Suitable pull request status labels are added (ready-for-code-review)

Additional Information

@hasalarootcode hasalarootcode self-assigned this Jul 10, 2025
Copilot AI review requested due to automatic review settings July 10, 2025 15:06
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds environment-aware logic to the profile menu’s sign-out flow to clear the tenant cookie only in enterprise mode and updates the enterprise pages submodule reference.

  • Import appModes and useGetEnvironment to detect the current deployment environment.
  • Wrap tenant‐cookie deletion in an ENTERPRISE environment check.
  • Bump the enterprise submodule commit in frontend/pages/enterprise.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
frontend/src/community/common/components/molecules/ProfileMenu/ProfileMenu.tsx Add environment hook and conditional tenant‐cookie clearing on sign out
frontend/pages/enterprise Update submodule commit reference
Comments suppressed due to low confidence (2)

frontend/src/community/common/components/molecules/ProfileMenu/ProfileMenu.tsx:17

  • [nitpick] Importing an enterprise-specific hook into a community component increases coupling; consider passing the environment flag down as a prop or abstracting this logic higher up to decouple modules.
import { useGetEnvironment } from "~enterprise/common/hooks/useGetEnvironment";

frontend/src/community/common/components/molecules/ProfileMenu/ProfileMenu.tsx:72

  • The new conditional logic for ENTERPRISE mode should have unit tests to verify that the tenant cookie is only cleared in enterprise environments.
    if (environment === appModes.ENTERPRISE) {


const handleSignOut = async () => {
if (environment === appModes.ENTERPRISE) {
document.cookie = `tenant=; domain=.${process.env.NEXT_PUBLIC_DOMAIN}; secure; path=/; SameSite=Lax`;
Copy link

Copilot AI Jul 10, 2025

Choose a reason for hiding this comment

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

To reliably delete the tenant cookie, include an expiration directive (e.g., expires=Thu, 01 Jan 1970 00:00:00 GMT or Max-Age=0) when setting it.

Suggested change
document.cookie = `tenant=; domain=.${process.env.NEXT_PUBLIC_DOMAIN}; secure; path=/; SameSite=Lax`;
document.cookie = `tenant=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=.${process.env.NEXT_PUBLIC_DOMAIN}; secure; path=/; SameSite=Lax`;

Copilot uses AI. Check for mistakes.
@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant