-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat: add favorite applications #41462
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release
Are you sure you want to change the base?
Changes from all commits
49de7ec
97bdca2
1c5a8e3
3787c90
a2c7716
d8329ad
0e383a4
5f2593e
557d9ca
c90366b
ac57c30
e00d7a2
01dc687
6976baf
6f9f36d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| import { | ||
| ReduxActionErrorTypes, | ||
| ReduxActionTypes, | ||
| } from "ee/constants/ReduxActionConstants"; | ||
| import type { ApplicationPayload } from "entities/Application"; | ||
|
|
||
| export const toggleFavoriteApplication = (applicationId: string) => ({ | ||
| type: ReduxActionTypes.TOGGLE_FAVORITE_APPLICATION_INIT, | ||
| payload: { applicationId }, | ||
| }); | ||
|
|
||
| export const toggleFavoriteApplicationSuccess = ( | ||
| applicationId: string, | ||
| isFavorited: boolean, | ||
| ) => ({ | ||
| type: ReduxActionTypes.TOGGLE_FAVORITE_APPLICATION_SUCCESS, | ||
| payload: { applicationId, isFavorited }, | ||
| }); | ||
|
|
||
| export const toggleFavoriteApplicationError = (applicationId: string) => ({ | ||
| type: ReduxActionErrorTypes.TOGGLE_FAVORITE_APPLICATION_ERROR, | ||
| payload: { applicationId }, | ||
| }); | ||
|
|
||
| export const fetchFavoriteApplications = () => ({ | ||
| type: ReduxActionTypes.FETCH_FAVORITE_APPLICATIONS_INIT, | ||
| }); | ||
|
|
||
| export const fetchFavoriteApplicationsSuccess = ( | ||
| applications: ApplicationPayload[], | ||
| ) => ({ | ||
| type: ReduxActionTypes.FETCH_FAVORITE_APPLICATIONS_SUCCESS, | ||
| payload: applications, | ||
| }); | ||
|
|
||
| export const fetchFavoriteApplicationsError = () => ({ | ||
| type: ReduxActionErrorTypes.FETCH_FAVORITE_APPLICATIONS_ERROR, | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ import type { | |
| WorkspaceUser, | ||
| WorkspaceUserRoles, | ||
| } from "ee/constants/workspaceConstants"; | ||
| import { FAVORITES_KEY } from "ee/constants/workspaceConstants"; | ||
| import type { Package } from "ee/constants/PackageConstants"; | ||
| import type { UpdateApplicationRequest } from "ee/api/ApplicationApi"; | ||
|
|
||
|
|
@@ -59,6 +60,30 @@ export const handlers = { | |
| ) => { | ||
| draftState.loadingStates.isFetchingApplications = false; | ||
| }, | ||
| // Handle favorites workspace - populate applications with favorite apps | ||
| [ReduxActionTypes.FETCH_FAVORITE_APPLICATIONS_INIT]: ( | ||
| draftState: SelectedWorkspaceReduxState, | ||
| ) => { | ||
| draftState.loadingStates.isFetchingApplications = true; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we updating
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. favorites is being treated like a regular workspace - so we reused this without creating a new loading state. If you really wanted, we could create a different state. Seemed simpler to keep one |
||
| }, | ||
| [ReduxActionTypes.FETCH_FAVORITE_APPLICATIONS_SUCCESS]: ( | ||
| draftState: SelectedWorkspaceReduxState, | ||
| action: ReduxAction<ApplicationPayload[]>, | ||
| ) => { | ||
| draftState.loadingStates.isFetchingApplications = false; | ||
|
|
||
| // Only replace applications when we're in the virtual favorites workspace. | ||
| // This prevents overwriting a real workspace's applications when favorites | ||
| // are fetched in the background. | ||
| if (draftState.workspace.id === FAVORITES_KEY) { | ||
| draftState.applications = action.payload; | ||
| } | ||
| }, | ||
| [ReduxActionErrorTypes.FETCH_FAVORITE_APPLICATIONS_ERROR]: ( | ||
| draftState: SelectedWorkspaceReduxState, | ||
| ) => { | ||
| draftState.loadingStates.isFetchingApplications = false; | ||
| }, | ||
| [ReduxActionTypes.DELETE_APPLICATION_SUCCESS]: ( | ||
| draftState: SelectedWorkspaceReduxState, | ||
| action: ReduxAction<ApplicationPayload>, | ||
|
|
@@ -242,6 +267,25 @@ export const handlers = { | |
| ) => { | ||
| draftState.loadingStates.isFetchingCurrentWorkspace = false; | ||
| }, | ||
| [ReduxActionTypes.TOGGLE_FAVORITE_APPLICATION_SUCCESS]: ( | ||
| draftState: SelectedWorkspaceReduxState, | ||
| action: ReduxAction<{ applicationId: string; isFavorited: boolean }>, | ||
| ) => { | ||
| const { applicationId, isFavorited } = action.payload; | ||
| const isFavoritesWorkspace = draftState.workspace.id === FAVORITES_KEY; | ||
|
|
||
| if (isFavoritesWorkspace && !isFavorited) { | ||
| draftState.applications = draftState.applications.filter( | ||
| (app) => (app.baseId || app.id) !== applicationId, | ||
| ); | ||
| } else { | ||
| draftState.applications = draftState.applications.map((app) => | ||
| (app.baseId || app.id) === applicationId | ||
| ? { ...app, isFavorited } | ||
| : app, | ||
| ); | ||
| } | ||
| }, | ||
| }; | ||
|
|
||
| const selectedWorkspaceReducer = createImmerReducer(initialState, handlers); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,10 @@ import WorkspaceApi from "ee/api/WorkspaceApi"; | |
| import type { ApiResponse } from "api/ApiResponses"; | ||
| import { getFetchedWorkspaces } from "ee/selectors/workspaceSelectors"; | ||
| import { getCurrentUser } from "selectors/usersSelectors"; | ||
| import { | ||
| DEFAULT_FAVORITES_WORKSPACE, | ||
| FAVORITES_KEY, | ||
| } from "ee/constants/workspaceConstants"; | ||
| import type { Workspace } from "ee/constants/workspaceConstants"; | ||
| import history from "utils/history"; | ||
| import { APPLICATIONS_URL } from "constants/routes"; | ||
|
|
@@ -62,6 +66,10 @@ export function* fetchAllWorkspacesSaga( | |
| payload: workspaces, | ||
| }); | ||
|
|
||
| // Fetch user's favorite applications to populate favoriteApplicationIds | ||
| // This ensures favorites are shown correctly across all workspaces | ||
| yield put({ type: ReduxActionTypes.FETCH_FAVORITE_APPLICATIONS_INIT }); | ||
|
|
||
| if (action?.payload?.workspaceId || action?.payload?.fetchEntities) { | ||
| yield call(fetchEntitiesOfWorkspaceSaga, action); | ||
| } | ||
|
|
@@ -82,6 +90,18 @@ export function* fetchEntitiesOfWorkspaceSaga( | |
| try { | ||
| const allWorkspaces: Workspace[] = yield select(getFetchedWorkspaces); | ||
| const workspaceId = action?.payload?.workspaceId || allWorkspaces[0]?.id; | ||
|
|
||
| // Handle virtual favorites workspace specially | ||
| if (workspaceId === FAVORITES_KEY) { | ||
| yield put({ | ||
| type: ReduxActionTypes.SET_CURRENT_WORKSPACE, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to explicitly set the current workspace here? I believe this would happen automatically when the workspace is clicked on the left pane |
||
| payload: DEFAULT_FAVORITES_WORKSPACE, | ||
| }); | ||
| yield put({ type: ReduxActionTypes.FETCH_FAVORITE_APPLICATIONS_INIT }); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| const activeWorkspace = allWorkspaces.find( | ||
| (workspace) => workspace.id === workspaceId, | ||
| ); | ||
|
|
@@ -338,10 +358,18 @@ export function* createWorkspaceSaga( | |
| yield call(resolve); | ||
| } | ||
|
|
||
| // get created workspace in focus | ||
| // Get created workspace in focus | ||
| // @ts-expect-error: response is of type unknown | ||
| const workspaceId = response.data.id; | ||
|
|
||
| // Refresh workspaces and entities for the newly created workspace so that | ||
| // the left panel and applications list reflect the new workspace instead of | ||
| // staying on the previous (e.g. Favorites) virtual workspace. | ||
| yield put({ | ||
| type: ReduxActionTypes.FETCH_ALL_WORKSPACES_INIT, | ||
| payload: { workspaceId, fetchEntities: true }, | ||
| }); | ||
|
|
||
|
Comment on lines
+365
to
+372
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't quite get the intent of this. Right now when we create a new workspace, user is redirected to the new workspace and you can see that happening in line 374. When the redirection happens we automatically fetch. So what problem are we fixing here and how does it get resolved by fetching all workspaces? |
||
| history.push(`${window.location.pathname}?workspaceId=${workspaceId}`); | ||
| } catch (error) { | ||
| yield call(reject, { _error: (error as Error).message }); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import SuperUserSagas from "ee/sagas/SuperUserSagas"; | |
| import organizationSagas from "ee/sagas/organizationSagas"; | ||
| import userSagas from "ee/sagas/userSagas"; | ||
| import workspaceSagas from "ee/sagas/WorkspaceSagas"; | ||
| import favoritesSagasListener from "sagas/FavoritesSagas"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do not create a new saga; since they are just virtual workspaces, move all your logic to workspaceSagas |
||
| import { watchPluginActionExecutionSagas } from "sagas/ActionExecution/PluginActionSaga"; | ||
| import { watchActionSagas } from "sagas/ActionSagas"; | ||
| import apiPaneSagas from "sagas/ApiPaneSagas"; | ||
|
|
@@ -115,4 +116,5 @@ export const sagas = [ | |
| gitSagas, | ||
| gitApplicationSagas, | ||
| PostEvaluationSagas, | ||
| favoritesSagasListener, | ||
| ]; | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential cause of intermittent navigation bug reported in PR comments.
When
hasFavoritesis true, the favorites workspace is prepended to the array. IfworkspaceIdFromQueryParamsis null/undefined, line 1188 defaults toworkspaces[0]?.id, which would be__favorites__.This may explain the reviewer's report of sometimes landing on Favorites instead of the expected workspace when navigating Home.
Consider preserving the user's last-visited real workspace or excluding the virtual workspace from the default selection:
// Inject virtual Favorites workspace at the top if user has favorites if (hasFavorites && !isFetchingWorkspaces) { const favoritesWorkspace = { id: "__favorites__", name: "Favorites", isVirtual: true, userPermissions: [], }; workspaces = [favoritesWorkspace, ...workspaces]; } + // Find first real workspace for default selection (skip virtual workspaces) + const defaultWorkspace = workspaces.find((ws: Workspace) => !ws.isVirtual); + const [activeWorkspaceId, setActiveWorkspaceId] = useState< string | undefined >( - workspaceIdFromQueryParams ? workspaceIdFromQueryParams : workspaces[0]?.id, + workspaceIdFromQueryParams ? workspaceIdFromQueryParams : defaultWorkspace?.id, );🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@salevine This callout seems legit; can you check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I resolved this