feat(router): Go to default workspace when clicking app logo #25706

cogk · 2024-03-28T14:10:07Z

closes #25690
relates to #25257

(cherry picked from commit e8fe24d)

…#25727) (cherry picked from commit e8fe24d) Co-authored-by: Corentin Flr <10946971+cogk@users.noreply.github.com>

# [15.21.0](v15.20.0...v15.21.0) (2024-04-09) ### Bug Fixes * add in_install flag to print_format validate (backport [#25752](#25752)) ([#25779](#25779)) ([b4eb918](b4eb918)) * auto add modified index when sort_field is set to it ([#25686](#25686)) ([#25786](#25786)) ([9e37908](9e37908)) * bigint validation (backport [#25733](#25733)) ([#25822](#25822)) ([4af75a1](4af75a1)) * check if user exists in browse command ([2b50e48](2b50e48)) * Column 'creation' in order clause was ambiguous ([97cc96e](97cc96e)) * copy paste from Excel (issue [#24371](#24371)) ([d816753](d816753)) * copy paste from Excel (issue [#24371](#24371)) ([a31c530](a31c530)) * **dashboard_chart:** use dict.get() to avoid a KeyError ([5fd854a](5fd854a)) * Don't assign returned values if row is deleted ([#25806](#25806)) ([#25826](#25826)) ([be3fd94](be3fd94)) * don't show bulk actions for doctype with workflow ([bf9ed2a](bf9ed2a)) * dont allow querying files to website users ([#25094](#25094)) ([#25701](#25701)) ([f95b4d6](f95b4d6)) * ensure we don't try to add int and NoneType ([8dcabaf](8dcabaf)) * Get filter value based on depends_on field ([#25861](#25861)) ([#25862](#25862)) ([dc113c4](dc113c4)) * **grid:** don't crash if row doesn't exist ([755d4a9](755d4a9)) * **grid:** ensure that `doc.name` is truthy before proceeding ([#25800](#25800)) ([#25829](#25829)) ([6234baf](6234baf)) * **layout:** handle `fieldobj` being null ([67eba0f](67eba0f)) * **link:** get_input_value returns `""` ([#25878](#25878)) ([#25883](#25883)) ([f918416](f918416)) * **ListView:** In ListView make visible link title value for Subject column ([#25569](#25569)) ([30737e5](30737e5)), closes [#25567](#25567) * log report errors to aid debugging ([#25738](#25738)) ([#25846](#25846)) ([ba8fd90](ba8fd90)) * make_request - prevents an error during response parsing if the response body is empty. ([#24613](#24613)) ([057db90](057db90)) * max-width of email attachment filename ([4b879f2](4b879f2)) * only try JSON if content-type says so ([#24936](#24936)) ([c5ddbb6](c5ddbb6)) * **sentry:** correctly skip `frappe.ValidationError` and its children ([e18ff5d](e18ff5d)) * type error in workflow ([#25847](#25847)) ([#25849](#25849)) ([32bbba3](32bbba3)) ### Features * don't require editing MariaDB configuration to setup frappe (backport [#25609](#25609)) ([#25757](#25757)) ([b47c658](b47c658)) * Include `before_print` in doctype event of Server Script (backport [#25858](#25858)) ([244be81](244be81)) * **router:** Go to default workspace when clicking app logo ([#25706](#25706)) ([#25727](#25727)) ([92919d1](92919d1)) ### Performance Improvements * render list (backport [#25524](#25524)) ([#25699](#25699)) ([d103b91](d103b91)) * **Scheduling:** add jitter to job scheduling ([#25857](#25857)) ([180c3b2](180c3b2)), closes [#19007](#19007)

ankush · 2024-04-10T12:47:06Z

frappe/public/js/frappe/router.js

+		}
+
+		// Workspace
+		let private_home = `home-${frappe.user.name.toLowerCase()}`;


This probably adds username twice, does it work for you?

To be fair, I don't know what the original code does, I never knew there where private home workspaces. I tried to keep it unchanged during this refactor, maybe I made a mistake somewhere?

Getting this error, I have no idea how this works either.

But:

I didn't anticipate that it was done again in convert_to_standard_route.

frappe/frappe/public/js/frappe/router.js

Lines 180 to 203 in caf7aec

let private_workspace = route[1] && `${route[1]}-${frappe.user.name.toLowerCase()}`;

if (frappe.workspaces[route[0]]) {

// public workspace

route = ["Workspaces", frappe.workspaces[route[0]].title];

} else if (route[0] == "private") {

// private workspace

if (!frappe.workspaces[private_workspace] && localStorage.new_workspace) {

let new_workspace = JSON.parse(localStorage.new_workspace);

if (frappe.router.slug(new_workspace.title) === route[1]) {

frappe.workspaces[private_workspace] = new_workspace;

}

}

if (!frappe.workspaces[private_workspace]) {

frappe.msgprint(__("Workspace <b>{0}</b> does not exist", [route[1]]));

return ["Workspaces"];

}

route = ["Workspaces", "private", frappe.workspaces[private_workspace].title];

} else if (this.routes[route[0]]) {

// route

route = await this.set_doctype_route(route);

}

return route;

Code would be cleaner if convert_to_standard_route didn't do shenanigans with private workspaces and localStorage 😅

Code would be cleaner if workspace name wasn't concoction of workspace + username. 😄

…25706)" This reverts commit e8fe24d.

ankush · 2024-04-10T13:40:35Z

frappe/public/js/frappe/router.js

+		if (frappe.boot.user.default_workspace) {
+			default_page = frappe.router.slug(frappe.boot.user.default_workspace.name);
+		} else if (frappe.workspaces[private_home]) {
+			default_page = private_home;


For now best fix is to not use private_home as url but workspace's slug

Something like this? (redirect to /app/home even if there is a home-myUser workspace)

Suggested change

default_page = private_home;

default_page = "home";

no it's /app/private/home

this won't still handle other private workspaces 💀

https://github.com/frappe/frappe/blob/develop/frappe/public/js/frappe/views/workspace/workspace.js#L354-L356

this won't work for private workspaces because of naming 💩

Well I understand the issue better now, now that you've opened your fix PR.

You see, I mainly work on a fork of Frappe where we already decoupled the name from the title of Workspaces… When I ported my change to Frappe I forgot to use title where I should have. Hopefully I didn't break too much stuff, right?

nah, this broke for few users with private home ig 🤣

feat(router): Go to default workspace when clicking app logo

83bbfc0

cogk requested review from a team and akhilnarang and removed request for a team March 28, 2024 14:10

ankush merged commit e8fe24d into frappe:develop Mar 29, 2024
24 of 25 checks passed

ankush added the backport version-15-hotfix Backport the PR to v15 label Mar 29, 2024

mergify bot pushed a commit that referenced this pull request Mar 29, 2024

feat(router): Go to default workspace when clicking app logo (#25706)

7795eed

(cherry picked from commit e8fe24d)

mergify bot mentioned this pull request Mar 29, 2024

feat(router): Go to default workspace when clicking app logo (backport #25706) #25727

Merged

ankush pushed a commit that referenced this pull request Apr 8, 2024

feat(router): Go to default workspace when clicking app logo (#25706) (…

92919d1

…#25727) (cherry picked from commit e8fe24d) Co-authored-by: Corentin Flr <10946971+cogk@users.noreply.github.com>

ankush reviewed Apr 10, 2024

View reviewed changes

ankush added a commit that referenced this pull request Apr 10, 2024

Revert "feat(router): Go to default workspace when clicking app logo (#…

607178d

…25706)" This reverts commit e8fe24d.

ankush mentioned this pull request Apr 10, 2024

Revert "feat(router): Go to default workspace when clicking app logo" #25903

Closed

ankush reviewed Apr 10, 2024

View reviewed changes

ankush mentioned this pull request Apr 10, 2024

fix: private workspace routing #25904

Merged

1 task

mergify bot mentioned this pull request Apr 11, 2024

fix: private workspace routing (backport #25904) #25921

Merged

1 task

github-actions bot locked as resolved and limited conversation to collaborators Apr 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(router): Go to default workspace when clicking app logo #25706

feat(router): Go to default workspace when clicking app logo #25706

cogk commented Mar 28, 2024

ankush Apr 10, 2024

cogk Apr 10, 2024

ankush Apr 10, 2024

cogk Apr 10, 2024

cogk Apr 10, 2024

ankush Apr 10, 2024

ankush Apr 10, 2024

cogk Apr 10, 2024 •

edited

ankush Apr 10, 2024

ankush Apr 10, 2024

ankush Apr 10, 2024 •

edited

cogk Apr 10, 2024

ankush Apr 10, 2024

	let private_workspace = route[1] && `${route[1]}-${frappe.user.name.toLowerCase()}`;

	if (frappe.workspaces[route[0]]) {
	// public workspace
	route = ["Workspaces", frappe.workspaces[route[0]].title];
	} else if (route[0] == "private") {
	// private workspace
	if (!frappe.workspaces[private_workspace] && localStorage.new_workspace) {
	let new_workspace = JSON.parse(localStorage.new_workspace);
	if (frappe.router.slug(new_workspace.title) === route[1]) {
	frappe.workspaces[private_workspace] = new_workspace;
	}
	}
	if (!frappe.workspaces[private_workspace]) {
	frappe.msgprint(__("Workspace <b>{0}</b> does not exist", [route[1]]));
	return ["Workspaces"];
	}
	route = ["Workspaces", "private", frappe.workspaces[private_workspace].title];
	} else if (this.routes[route[0]]) {
	// route
	route = await this.set_doctype_route(route);
	}

	return route;

feat(router): Go to default workspace when clicking app logo #25706

feat(router): Go to default workspace when clicking app logo #25706

Conversation

cogk commented Mar 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cogk Apr 10, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ankush Apr 10, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cogk Apr 10, 2024 •

edited

ankush Apr 10, 2024 •

edited