group-settings: Handle invalid urls correctly.

There were couple of problems in our handling of invalid or
incomplete URLs-
- The browser back button behavior breaks if someone enters
url with invalid group ID, incorrect group name.
- On typing group edit URLs with invalid group ID, an error
was raised and the URLs remained the same with just opening
the groups overlay with "Your" tab selected in left panel.
- On typing group edit URLs with incorrect right side tab or
without any right side tab, we showed "general" section, which
is fine, but the URL was still incorrect.

This commit fixes the above mentioned problems-
- URLs with invalid group IDs are now handled gracefully
with groups UI opening in the same way as before but the
url is updated to "#groups/your".
- We now update the right side tab to "general" if right
side tab is invalid or there is no right side tab.
- All the URL updates to fix invalid urls are done using
"history.replaceState" to make sure the browser back button
behaves as expected.
- All the code for checking the urls is done in hashchange.js
itself, so we can remove some code from change_state.
This commit is contained in:
Sahil Batra
2024-02-09 19:04:09 +05:30
committed by Tim Abbott
parent eea5ee8923
commit 2fe36cc0d7
5 changed files with 54 additions and 22 deletions

View File

@@ -8,6 +8,7 @@ import * as settings_data from "./settings_data";
import * as stream_data from "./stream_data"; import * as stream_data from "./stream_data";
import * as sub_store from "./sub_store"; import * as sub_store from "./sub_store";
import type {StreamSubscription} from "./sub_store"; import type {StreamSubscription} from "./sub_store";
import * as user_groups from "./user_groups";
import type {UserGroup} from "./user_groups"; import type {UserGroup} from "./user_groups";
// TODO(typescript): Move to filter.js when it's converted to typescript. // TODO(typescript): Move to filter.js when it's converted to typescript.
@@ -238,3 +239,36 @@ export function validate_stream_settings_hash(hash: string): string {
} }
return hash; return hash;
} }
export function validate_group_settings_hash(hash: string): string {
const hash_components = hash.slice(1).split(/\//);
const section = hash_components[1];
if (/\d+/.test(section)) {
const group_id = Number.parseInt(section, 10);
const group = user_groups.maybe_get_user_group_from_id(group_id);
if (!group) {
// Some users can type random url of the form
// /#groups/<random-group-id> we need to handle that.
return "#groups/your";
}
const group_name = hash_components[2];
let right_side_tab = hash_components[3];
const valid_right_side_tab_values = new Set(["general", "members"]);
if (group.name === group_name && valid_right_side_tab_values.has(right_side_tab)) {
return hash;
}
if (!valid_right_side_tab_values.has(right_side_tab)) {
right_side_tab = "general";
}
return group_edit_url(group, right_side_tab);
}
const valid_section_values = ["new", "your", "all"];
if (!valid_section_values.includes(section)) {
blueslip.info("invalid section for groups: " + section);
return "#groups/your";
}
return hash;
}

View File

@@ -248,8 +248,12 @@ function do_hashchange_overlay(old_hash) {
} }
} }
if (base === "groups" && !section) { if (base === "groups") {
history.replaceState(null, "", browser_history.get_full_url("groups/your")); const valid_hash = hash_util.validate_group_settings_hash(window.location.hash);
if (valid_hash !== window.location.hash) {
history.replaceState(null, "", browser_history.get_full_url(valid_hash));
section = hash_parser.get_current_hash_section();
}
} }
// Start by handling the specific case of going // Start by handling the specific case of going

View File

@@ -568,33 +568,20 @@ export function change_state(section, right_side_tab) {
return; return;
} }
if (section === "your") {
group_list_toggler.goto("your-groups");
empty_right_panel();
return;
}
// if the section is a valid number. // if the section is a valid number.
if (/\d+/.test(section)) { if (/\d+/.test(section)) {
const group_id = Number.parseInt(section, 10); const group_id = Number.parseInt(section, 10);
const group = user_groups.get_user_group_from_id(group_id); const group = user_groups.get_user_group_from_id(group_id);
if (!group) { show_right_section();
// Some users can type random url of the form // We show the list of user groups in the left panel
// /#groups/<random-group-id> we need to handle that. // based on the tab that is active. It is `your-groups`
group_list_toggler.goto("your-groups"); // tab by default.
} else { redraw_user_group_list();
show_right_section(); select_tab = right_side_tab;
// We show the list of user groups in the left panel switch_to_group_row(group);
// based on the tab that is active. It is `your-groups`
// tab by default.
redraw_user_group_list();
select_tab = right_side_tab;
switch_to_group_row(group);
}
return; return;
} }
blueslip.info("invalid section for groups: " + section);
group_list_toggler.goto("your-groups"); group_list_toggler.goto("your-groups");
empty_right_panel(); empty_right_panel();
} }

View File

@@ -67,6 +67,10 @@ export function get_user_group_from_id(group_id: number): UserGroup {
return user_group; return user_group;
} }
export function maybe_get_user_group_from_id(group_id: number): UserGroup | undefined {
return user_group_by_id_dict.get(group_id);
}
export function update(event: UserGroupUpdateEvent): void { export function update(event: UserGroupUpdateEvent): void {
const group = get_user_group_from_id(event.group_id); const group = get_user_group_from_id(event.group_id);
if (event.data.name !== undefined) { if (event.data.name !== undefined) {

View File

@@ -49,6 +49,9 @@ run_test("user_groups", () => {
user_groups.add(admins); user_groups.add(admins);
assert.deepEqual(user_groups.get_user_group_from_id(admins.id), admins); assert.deepEqual(user_groups.get_user_group_from_id(admins.id), admins);
assert.equal(user_groups.maybe_get_user_group_from_id(99), undefined);
assert.deepEqual(user_groups.get_user_group_from_id(admins.id), admins);
const update_name_event = { const update_name_event = {
group_id: admins.id, group_id: admins.id,
data: { data: {