fix(groups): Fix UI encoding of groups with spaces in urns (#4021)

This commit is contained in:
John Joyce 2022-02-01 10:47:45 -08:00 committed by GitHub
parent 928ab74f33
commit ea271711bb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 63 additions and 73 deletions

View File

@ -15,6 +15,7 @@ import com.linkedin.metadata.utils.GenericAspectUtils;
import com.linkedin.mxe.MetadataChangeProposal;
import graphql.schema.DataFetcher;
import graphql.schema.DataFetchingEnvironment;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import static com.linkedin.datahub.graphql.resolvers.ResolverUtils.*;
@ -42,7 +43,8 @@ public class CreateGroupResolver implements DataFetcher<CompletableFuture<String
// First, check if the group already exists.
// Create the Group key.
final CorpGroupKey key = new CorpGroupKey();
key.setName(input.getName());
final String id = input.getId() != null ? input.getId() : UUID.randomUUID().toString();
key.setName(id); // 'name' in the key really reflects nothing more than a stable "id".
// Create the Group info.
final CorpGroupInfo info = new CorpGroupInfo();

View File

@ -4467,6 +4467,11 @@ type AuditStamp {
Input for creating a new group
"""
input CreateGroupInput {
"""
Optional! A custom id to use as the primary key identifier for the group. If not provided, a random UUID will be generated as the id.
"""
id: String
"""
The display name of the group
"""

View File

@ -7,6 +7,7 @@ import useIsLineageMode from '../lineage/utils/useIsLineageMode';
import { SearchablePage } from '../search/SearchablePage';
import { useEntityRegistry } from '../useEntityRegistry';
import analytics, { EventType } from '../analytics';
import { decodeUrn } from './shared/utils';
interface RouteParams {
urn: string;
@ -21,7 +22,7 @@ interface Props {
*/
export const EntityPage = ({ entityType }: Props) => {
const { urn: encodedUrn } = useParams<RouteParams>();
const urn = decodeURIComponent(encodedUrn);
const urn = decodeUrn(encodedUrn);
const entityRegistry = useEntityRegistry();
const isBrowsable = entityRegistry.getEntity(entityType).isBrowseEnabled();
const isLineageSupported = entityRegistry.getEntity(entityType).isLineageEnabled();

View File

@ -53,7 +53,7 @@ export class GroupEntity implements Entity<CorpGroup> {
urn={data.urn}
name={data.info?.displayName || data.name || ''}
description={data.info?.description}
membersCount={data?.relationships?.total || 0}
membersCount={(data as any)?.memberCount?.total || 0}
/>
);

View File

@ -10,6 +10,7 @@ import { Message } from '../../shared/Message';
import GroupMembers from './GroupMembers';
import { LegacyEntityProfile } from '../../shared/LegacyEntityProfile';
import { useEntityRegistry } from '../../useEntityRegistry';
import { decodeUrn } from '../shared/utils';
const messageStyle = { marginTop: '10%' };
@ -27,7 +28,8 @@ const MEMBER_PAGE_SIZE = 20;
*/
export default function GroupProfile() {
const entityRegistry = useEntityRegistry();
const { urn } = useUserParams();
const { urn: encodedUrn } = useUserParams();
const urn = encodedUrn && decodeUrn(encodedUrn);
const { loading, error, data } = useGetGroupQuery({ variables: { urn, membersCount: MEMBER_PAGE_SIZE } });
const ownershipResult = useGetAllEntitySearchResults({

View File

@ -2,7 +2,9 @@ export function urlEncodeUrn(urn: string) {
return (
urn &&
urn
.replace(/%/g, '%25')
// Hack - React Router v5 does not like pre-url-encoded paths. Since URNs can contain free form IDs, there's nothing preventing them from having percentages.
// If we use double encoded paths, React ends up decoding them fully, which breaks our ability to read urns properly.
.replace(/%/g, '{{encoded_percent}}')
.replace(/\//g, '%2F')
.replace(/\?/g, '%3F')
.replace(/#/g, '%23')
@ -11,6 +13,12 @@ export function urlEncodeUrn(urn: string) {
);
}
export function decodeUrn(encodedUrn: string) {
// Hack-This is not ideal because it means that if you had the percent
// sequence in your urn things may not work as expected.
return decodeURIComponent(encodedUrn).replace(/{{encoded_percent}}/g, '%');
}
export function getNumberWithOrdinal(n) {
const suffixes = ['th', 'st', 'nd', 'rd'];
const v = n % 100;

View File

@ -5,6 +5,7 @@ import { Tag, EntityType, SearchResult } from '../../../types.generated';
import DefaultPreviewCard from '../../preview/DefaultPreviewCard';
import { Entity, IconStyleType, PreviewType } from '../Entity';
import { getDataForEntityType } from '../shared/containers/profile/utils';
import { urlEncodeUrn } from '../shared/utils';
import TagProfile from './TagProfile';
const PreviewTagIcon = styled(TagOutlined)`
@ -56,7 +57,7 @@ export class TagEntity implements Entity<Tag> {
<DefaultPreviewCard
description={data.description || ''}
name={data.name}
url={`/${this.getPathName()}/${data.urn}`}
url={`/${this.getPathName()}/${urlEncodeUrn(data.urn)}`}
logoComponent={<PreviewTagIcon />}
type="Tag"
/>

View File

@ -11,6 +11,7 @@ import { navigateToSearchUrl } from '../../search/utils/navigateToSearchUrl';
import { Message } from '../../shared/Message';
import { AvatarsGroup } from '../../shared/avatar';
import { useEntityRegistry } from '../../useEntityRegistry';
import { decodeUrn } from '../shared/utils';
const PageContainer = styled.div`
padding: 32px 100px;
@ -79,19 +80,15 @@ type TagPageParams = {
*/
export default function TagProfile() {
const { urn: encodedUrn } = useParams<TagPageParams>();
const urn = decodeURIComponent(encodedUrn);
const urn = decodeUrn(encodedUrn);
const { loading, error, data } = useGetTagQuery({ variables: { urn } });
const entityRegistry = useEntityRegistry();
const history = useHistory();
console.log(data);
const entityAndSchemaQuery = `tags:"${data?.tag?.name}" OR fieldTags:"${data?.tag?.name}" OR editedFieldTags:"${data?.tag?.name}"`;
const entityQuery = `tags:"${data?.tag?.name}"`;
console.log(entityAndSchemaQuery);
const allSearchResultsByType = useGetAllEntitySearchResults({
query: entityAndSchemaQuery,
start: 0,

View File

@ -10,6 +10,7 @@ import { LegacyEntityProfile } from '../../shared/LegacyEntityProfile';
import { CorpUser, EntityType, SearchResult, EntityRelationshipsResult } from '../../../types.generated';
import UserGroups from './UserGroups';
import { useEntityRegistry } from '../../useEntityRegistry';
import { decodeUrn } from '../shared/utils';
const messageStyle = { marginTop: '10%' };
@ -25,7 +26,8 @@ const GROUP_PAGE_SIZE = 20;
* Responsible for reading & writing users.
*/
export default function UserProfile() {
const { urn } = useUserParams();
const { urn: encodedUrn } = useUserParams();
const urn = decodeUrn(encodedUrn);
const { loading, error, data } = useGetUserQuery({ variables: { urn, groupsCount: GROUP_PAGE_SIZE } });
const entityRegistry = useEntityRegistry();
const username = data?.corpUser?.username;

View File

@ -1,5 +1,5 @@
import React, { useState } from 'react';
import { message, Button, Input, Modal, Typography, Form } from 'antd';
import { message, Button, Input, Modal, Typography, Form, Collapse } from 'antd';
import { useCreateGroupMutation } from '../../../graphql/group.generated';
type Props = {
@ -11,12 +11,14 @@ type Props = {
export default function CreateGroupModal({ visible, onClose, onCreate }: Props) {
const [stagedName, setStagedName] = useState('');
const [stagedDescription, setStagedDescription] = useState('');
const [stagedId, setStagedId] = useState<string | undefined>(undefined);
const [createGroupMutation] = useCreateGroupMutation();
const onCreateGroup = () => {
createGroupMutation({
variables: {
input: {
id: stagedId,
name: stagedName,
description: stagedDescription,
},
@ -71,6 +73,23 @@ export default function CreateGroupModal({ visible, onClose, onCreate }: Props)
onChange={(event) => setStagedDescription(event.target.value)}
/>
</Form.Item>
<Collapse ghost>
<Collapse.Panel header={<Typography.Text type="secondary">Advanced</Typography.Text>} key="1">
<Form.Item label={<Typography.Text strong>Group Id</Typography.Text>}>
<Typography.Paragraph>
By default, a random UUID will be generated to uniquely identify this group. If
you&apos;d like to provide a custom id instead to more easily keep track of this group,
you may provide it here. Be careful, you cannot easily change the group id after
creation.
</Typography.Paragraph>
<Input
placeholder="product_engineering"
value={stagedId || ''}
onChange={(event) => setStagedId(event.target.value)}
/>
</Form.Item>
</Collapse.Panel>
</Collapse>
</Form>
</Modal>
);

View File

@ -65,7 +65,7 @@ export default function GroupListItem({ group, onDelete }: Props) {
return (
<List.Item>
<GroupItemContainer>
<Link to={entityRegistry.getEntityUrl(EntityType.CorpGroup, group.urn)}>
<Link to={`${entityRegistry.getEntityUrl(EntityType.CorpGroup, group.urn)}`}>
<GroupHeaderContainer>
<CustomAvatar size={32} name={displayName} />
<div style={{ marginLeft: 16, marginRight: 16 }}>

View File

@ -1,6 +1,6 @@
import * as React from 'react';
import { Route, Switch, useRouteMatch, useLocation } from 'react-router-dom';
import { useHistory, Redirect } from 'react-router';
import { Redirect, useHistory } from 'react-router';
import { Tabs } from 'antd';
import { TabsProps } from 'antd/lib/tabs';
@ -24,13 +24,11 @@ export const RoutedTabs = ({ defaultPath, tabs, onTabChange, ...props }: Props)
const { path, url } = useRouteMatch();
const { pathname } = useLocation();
const history = useHistory();
const subRoutes = tabs.map((tab) => tab.path.replace('/', ''));
const trimmedPathName = pathname.endsWith('/') ? pathname.slice(0, pathname.length - 1) : pathname;
const splitPathName = trimmedPathName.split('/');
const providedPath = splitPathName[splitPathName.length - 1];
const activePath = subRoutes.includes(providedPath) ? providedPath : defaultPath.replace('/', '');
return (
<div>
<Tabs
@ -49,6 +47,7 @@ export const RoutedTabs = ({ defaultPath, tabs, onTabChange, ...props }: Props)
<Route exact path={path}>
<Redirect to={`${pathname}${pathname.endsWith('/') ? '' : '/'}${defaultPath}`} />
</Route>
{tabs.map((tab) => (
<Route
exact

View File

@ -55,28 +55,8 @@ fragment entityPreview on Entity {
displayName
description
}
relationships(input: { types: ["IsMemberOfGroup"], direction: INCOMING }) {
relationships {
type
direction
entity {
urn
type
... on CorpUser {
username
info {
active
displayName
title
firstName
lastName
}
editableInfo {
pictureLink
}
}
}
}
memberCount: relationships(input: { types: ["IsMemberOfGroup"], direction: INCOMING, start: 0, count: 1 }) {
total
}
}
... on Dashboard {

View File

@ -77,28 +77,10 @@ fragment searchResults on SearchResults {
displayName
description
}
relationships(input: { types: ["IsMemberOfGroup"], direction: INCOMING }) {
relationships {
type
direction
entity {
urn
type
... on CorpUser {
username
info {
active
displayName
title
firstName
lastName
}
editableInfo {
pictureLink
}
}
}
}
memberCount: relationships(
input: { types: ["IsMemberOfGroup"], direction: INCOMING, start: 0, count: 1 }
) {
total
}
}
... on Dashboard {

View File

@ -7,11 +7,8 @@ import com.linkedin.data.template.RecordTemplate;
import com.linkedin.metadata.models.AspectSpec;
import com.linkedin.mxe.MetadataChangeLog;
import com.linkedin.mxe.MetadataChangeProposal;
import java.io.UnsupportedEncodingException;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
import javax.annotation.Nonnull;
@ -103,14 +100,8 @@ public class EntityKeyUtils {
final DataMap dataMap = new DataMap();
for (int i = 0; i < urn.getEntityKey().getParts().size(); i++) {
final String urnPart = urn.getEntityKey().get(i);
try {
final String decodedUrnPart = URLDecoder.decode(urnPart, StandardCharsets.UTF_8.toString());
final RecordDataSchema.Field field = keySchema.getFields().get(i);
dataMap.put(field.getName(), decodedUrnPart);
} catch (UnsupportedEncodingException e) {
throw new RuntimeException(
String.format("Failed to convert URN to Entity Key. Unable to URL decoded urn part %s", urnPart), e);
}
final RecordDataSchema.Field field = keySchema.getFields().get(i);
dataMap.put(field.getName(), urnPart);
}
// #3. Finally, instantiate the record template with the newly created DataMap.

View File

@ -1169,6 +1169,7 @@ def test_create_group(frontend_session):
createGroup(input: $input) }""",
"variables": {
"input": {
"id": "test-id",
"name": "Test Group",
"description": "My test group"
}
@ -1190,7 +1191,7 @@ def test_create_group(frontend_session):
}\n
}""",
"variables": {
"urn": "urn:li:corpGroup:Test Group"
"urn": "urn:li:corpGroup:test-id"
}
}
response = frontend_session.post(