Skip to content
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

refactor(search): remove isSearchable, index conditions #914

Merged
merged 3 commits into from
Nov 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions scripts/migrate_url_to_user.sql
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
-- isFile column
-- 07 Oct 2020 @LoneRifle: Update function's url_history insertion step to include compulsory
-- isSearchable column
-- 16 Nov 2020 @LoneRifle: Update function's url_history insertion step to remove
-- isSearchable column
-- =============================================
CREATE OR REPLACE FUNCTION migrate_url_to_user(short_url_value text, to_user_email text) RETURNS void AS
$BODY$
Expand Down Expand Up @@ -45,8 +47,8 @@ BEGIN
RAISE EXCEPTION 'No transferring of links to the same user';
END IF;
-- Insert the intended changes into URL history table
INSERT INTO url_histories ("urlShortUrl","longUrl","state","userId","isFile","isSearchable","createdAt","updatedAt")
SELECT "shortUrl", "longUrl", "state", "to_user_id", "isFile", "isSearchable", CURRENT_TIMESTAMP, CURRENT_TIMESTAMP
INSERT INTO url_histories ("urlShortUrl","longUrl","state","userId","isFile","createdAt","updatedAt")
SELECT "shortUrl", "longUrl", "state", "to_user_id", "isFile", CURRENT_TIMESTAMP, CURRENT_TIMESTAMP
FROM urls
WHERE "shortUrl" = short_url LIMIT 1;
-- Update the link in the URL table
Expand Down
6 changes: 4 additions & 2 deletions scripts/migrate_user_links.sql
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
-- compulsory isFile column
-- 07 Oct 2020 @LoneRifle: Update function's url_history insertion step to include
-- compulsory isSearchable column
-- 16 Nov 2020 @LoneRifle: Update function's url_history insertion step to remove
-- isSearchable column
-- =============================================
CREATE OR REPLACE FUNCTION migrate_user_links(from_user_email text, to_user_email text) RETURNS void AS
$BODY$
Expand All @@ -41,8 +43,8 @@ BEGIN
RAISE EXCEPTION 'No transferring of links to the same user';
END IF;
-- Insert the intended changes into URL history table
INSERT INTO url_histories ("urlShortUrl","longUrl","state","userId","isFile","isSearchable","createdAt","updatedAt")
SELECT "shortUrl", "longUrl", "state", "to_user_id", "isFile", "isSearchable", CURRENT_TIMESTAMP, CURRENT_TIMESTAMP
INSERT INTO url_histories ("urlShortUrl","longUrl","state","userId","isFile","createdAt","updatedAt")
SELECT "shortUrl", "longUrl", "state", "to_user_id", "isFile", CURRENT_TIMESTAMP, CURRENT_TIMESTAMP
FROM urls
WHERE "userId" = from_user_id;
-- Update the links in the URL table
Expand Down
24 changes: 0 additions & 24 deletions src/client/user/actions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -491,29 +491,6 @@ const toggleUrlState = (shortUrl: string, state: UrlState) => (
})
}

const toggleIsSearchable = (shortUrl: string, isSearchable: boolean) => (
dispatch: ThunkDispatch<
GoGovReduxState,
void,
SetErrorMessageAction | SetSuccessMessageAction
>,
) => {
patch('/api/user/url', { shortUrl, isSearchable }).then((response) => {
if (response.ok) {
dispatch<void>(getUrlsForUser())
dispatch<SetSuccessMessageAction>(
rootActions.setSuccessMessage('Your link visibility has been updated.'),
)
return null
}

return response.json().then((json) => {
dispatch<SetErrorMessageAction>(rootActions.setErrorMessage(json.message))
return null
})
})
}

const openCreateUrlModal: () => OpenCreateUrlModalAction = () => ({
type: OPEN_CREATE_URL_MODAL,
})
Expand Down Expand Up @@ -733,7 +710,6 @@ export default {
setCreateShortLinkError,
setUrlFilter,
replaceFile,
toggleIsSearchable,
setEditedContactEmail,
setEditedDescription,
getUserMessage,
Expand Down
24 changes: 0 additions & 24 deletions src/client/user/components/CreateUrlModal/AddDescriptionForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ export default function AddDescriptionForm() {
const [isContactEmailValid, setIsContactEmailValid] = useState(true)
const [description, setDescription] = useState('')
const [isDescriptionValid, setIsDescriptionValid] = useState(true)
const [isSearchable, setIsSearchable] = useState(true)
const onContactEmailChange = (event: React.ChangeEvent<HTMLInputElement>) =>
setContactEmail(event.target.value)
const onDescriptionChange = (event: React.ChangeEvent<HTMLInputElement>) =>
Expand Down Expand Up @@ -74,27 +73,6 @@ export default function AddDescriptionForm() {
const jsonResponse = await response.json()
dispatch(rootActions.setErrorMessage(jsonResponse.message))
}
const toggleLinkSearchable = async (
event: React.ChangeEvent<HTMLInputElement>,
) => {
const newIsSearchable = event.target.checked
const response = await patch('/api/user/url', {
isSearchable: newIsSearchable,
shortUrl,
})

if (response.ok) {
dispatch(userActions.getUrlsForUser())
dispatch(
rootActions.setSuccessMessage('Your link visibility has been updated.'),
)
setIsSearchable(newIsSearchable)
return
}

const jsonResponse = await response.json()
dispatch(rootActions.setErrorMessage(jsonResponse.message))
}

const isBothFieldsBlank = contactEmail === '' && description === ''
const isContainsInvalidField = !(isContactEmailValid && isDescriptionValid)
Expand All @@ -116,8 +94,6 @@ export default function AddDescriptionForm() {
onDescriptionChange={onDescriptionChange}
onContactEmailValidation={setIsContactEmailValid}
onDescriptionValidation={setIsDescriptionValid}
isSearchable={isSearchable}
onIsSearchableChange={toggleLinkSearchable}
isMountedOnCreateUrlModal
/>
<div className={classes.buttonWrapper}>
Expand Down
3 changes: 0 additions & 3 deletions src/client/user/components/Drawer/ControlPanel/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,6 @@ export default function ControlPanel() {
} = useShortLink(drawerStates.relevantShortLink!)

// Manage values in our text fields.
const isSearchable = shortLinkState?.isSearchable || false
const originalLongUrl = removeHttpsProtocol(shortLinkState?.longUrl || '')
const editedLongUrl = shortLinkState?.editedLongUrl || ''
const editedContactEmail = shortLinkState?.editedContactEmail || ''
Expand Down Expand Up @@ -445,10 +444,8 @@ export default function ControlPanel() {
<div className={classes.inactiveDesc}>
<Divider className={classes.dividerInformation} />
<LinkInfoEditor
isSearchable={isSearchable}
contactEmail={editedContactEmail}
description={editedDescription}
onIsSearchableChange={(event) => shortLinkDispatch?.toggleIsSearchable(event.target.checked)}
onContactEmailChange={(event) => shortLinkDispatch?.setEditContactEmail(event.target.value)}
onDescriptionChange={(event) =>
shortLinkDispatch?.setEditDescription(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ export default function useShortLink(shortLink: string) {
const dispatchOptions = {
toggleStatus: () =>
dispatch(userActions.toggleUrlState(shortLink, urlState.state)),
toggleIsSearchable: (isSearchable: boolean) => {
dispatch(userActions.toggleIsSearchable(shortLink, isSearchable))
},
setEditLongUrl: (editedUrl: string) => {
dispatch(userActions.setEditedLongUrl(shortLink, editedUrl))
},
Expand Down
1 change: 0 additions & 1 deletion src/client/user/reducers/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ export type UrlType = {
state: UrlState
updatedAt: string
userId: number
isSearchable: boolean
description: string
editedDescription: string
contactEmail: string
Expand Down
32 changes: 0 additions & 32 deletions src/client/user/widgets/LinkInfoEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import BetaTag from '../../app/components/widgets/BetaTag'
import CollapsibleMessage from '../../app/components/CollapsibleMessage'
import ConfigOption, { TrailingPosition } from './ConfigOption'
import PrefixableTextField from './PrefixableTextField'
import GoSwitch from './GoSwitch'
import {
CollapsibleMessagePosition,
CollapsibleMessageType,
Expand Down Expand Up @@ -70,10 +69,8 @@ const useStyles = makeStyles((theme) =>
)

type LinkInfoEditorProps = {
isSearchable: boolean
contactEmail: string
description: string
onIsSearchableChange: (event: React.ChangeEvent<HTMLInputElement>) => void
onContactEmailChange: (event: React.ChangeEvent<HTMLInputElement>) => void
onDescriptionChange: (event: React.ChangeEvent<HTMLInputElement>) => void
onContactEmailValidation: (isContactEmailValid: boolean) => void
Expand All @@ -82,10 +79,8 @@ type LinkInfoEditorProps = {
}

export default function LinkInfoEditor({
isSearchable,
contactEmail,
description,
onIsSearchableChange,
onContactEmailChange,
onDescriptionChange,
onContactEmailValidation,
Expand Down Expand Up @@ -162,33 +157,6 @@ export default function LinkInfoEditor({
className={classes.linkInformationHeader}
color="primary"
>
<ConfigOption
title={
isSearchable
? (
<>
Your link is <span className={classes.activeText}>visible</span> in
GoSearch results
</>
)
: (
<>
Your link is <span className={classes.inactiveText}>not visible</span> in
GoSearch results
</>
)
}
titleVariant="h6"
titleClassName={isMobileView ? classes.regularText : ''}
trailing={
<GoSwitch
color="primary"
checked={isSearchable}
onChange={onIsSearchableChange}
/>
}
trailingPosition={TrailingPosition.center}
/>
</Typography>
<ConfigOption
title={contactEmailHelp}
Expand Down
1 change: 0 additions & 1 deletion src/server/api/user/validators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ export const urlEditSchema = Joi.object({
file: Joi.object().keys().required(),
}),
state: Joi.string().allow(ACTIVE, INACTIVE).only(),
isSearchable: Joi.boolean(),
description: Joi.string()
.allow('')
.max(LINK_DESCRIPTION_MAX_LENGTH)
Expand Down
2 changes: 0 additions & 2 deletions src/server/controllers/UserController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ export class UserController implements UserControllerInterface {
longUrl,
shortUrl,
state,
isSearchable,
description,
contactEmail,
}: UrlEditRequest = req.body
Expand Down Expand Up @@ -126,7 +125,6 @@ export class UserController implements UserControllerInterface {
longUrl,
state: urlState,
file,
isSearchable,
contactEmail: newContactEmail,
description: description?.trim(),
})
Expand Down
22 changes: 22 additions & 0 deletions src/server/db_migrations/20201116_remove_isSearchable.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
-- This migration script removes Urls.isSearchable with a default
-- of true and changes the index to index unconditionally
-- sql statements in scripts/ should be run to update functions too
BEGIN TRANSACTION;

-- Set "isSearchable" in url_histories to be false
ALTER TABLE url_histories DROP COLUMN "isSearchable";
ALTER TABLE urls DROP COLUMN "isSearchable";


DROP INDEX IF EXISTS urls_weighted_search_idx;

-- Search will be run on a concatenation of vectors formed from short links and their
-- description. Descriptions are given a lower weight than short links as short link
-- words can be taken as the title and words there are likely to be more important than
-- those in their corresponding description.
-- Search queries will have to use this exact expresion to be able to utilize the index.
CREATE INDEX urls_weighted_search_idx ON urls USING gin ((setweight(to_tsvector(
'english', urls."shortUrl"), 'A') || setweight(to_tsvector('english',
urls."description"), 'B')));

COMMIT;
1 change: 0 additions & 1 deletion src/server/mappers/UrlMapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ export class UrlMapper implements Mapper<StorableUrl, UrlType> {
isFile: urlType.isFile,
createdAt: urlType.createdAt,
updatedAt: urlType.updatedAt,
isSearchable: urlType.isSearchable,
description: urlType.description,
contactEmail: urlType.contactEmail,
}
Expand Down
2 changes: 1 addition & 1 deletion src/server/models/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ export const urlSearchVector = `
setweight(to_tsvector('english', urls."description"), 'B')
`

export const urlSearchConditions = `urls.state = '${StorableUrlState.Active}' AND urls."isSearchable"=true`
export const urlSearchConditions = `urls.state = '${StorableUrlState.Active}' AND urls.description != ''`
21 changes: 1 addition & 20 deletions src/server/models/url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ interface UrlBaseType extends IdType {
readonly longUrl: string
readonly state: StorableUrlState
readonly isFile: boolean
readonly isSearchable: boolean
readonly contactEmail: string | null
readonly description: string
}
Expand Down Expand Up @@ -102,11 +101,6 @@ export const Url = <UrlTypeStatic>sequelize.define(
type: Sequelize.BOOLEAN,
allowNull: false,
},
isSearchable: {
type: Sequelize.BOOLEAN,
allowNull: false,
LoneRifle marked this conversation as resolved.
Show resolved Hide resolved
defaultValue: true,
},
contactEmail: {
type: Sequelize.TEXT,
allowNull: true,
Expand Down Expand Up @@ -173,15 +167,7 @@ export const Url = <UrlTypeStatic>sequelize.define(
name: 'urls_weighted_search_idx',
unique: false,
using: 'GIN',
fields: [
// Type definition on sequelize seems to be inaccurate.
// @ts-ignore
Sequelize.literal(`(${urlSearchVector})`),
],
where: {
state: ACTIVE,
isSearchable: true,
},
fields: [Sequelize.literal(`(${urlSearchVector})`)],
},
],
},
Expand Down Expand Up @@ -214,10 +200,6 @@ export const UrlHistory = <UrlHistoryStatic>sequelize.define('url_history', {
type: Sequelize.BOOLEAN,
allowNull: false,
},
isSearchable: {
type: Sequelize.BOOLEAN,
allowNull: false,
},
contactEmail: {
type: Sequelize.TEXT,
allowNull: true,
Expand All @@ -244,7 +226,6 @@ const writeToUrlHistory = async (
urlShortUrl: urlObj.shortUrl,
longUrl: urlObj.longUrl,
isFile: urlObj.isFile,
isSearchable: urlObj.isSearchable,
contactEmail: urlObj.contactEmail,
description: urlObj.description,
},
Expand Down
1 change: 0 additions & 1 deletion src/server/repositories/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ export type StorableUrl = Pick<
| 'isFile'
| 'createdAt'
| 'updatedAt'
| 'isSearchable'
| 'description'
| 'contactEmail'
>
Expand Down
11 changes: 2 additions & 9 deletions src/server/services/UrlManagementService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,7 @@ export class UrlManagementService implements UrlManagementServiceInterface {
shortUrl: string,
options: UpdateUrlOptions,
) => Promise<StorableUrl> = async (userId, shortUrl, options) => {
const {
state,
longUrl,
file,
isSearchable,
description,
contactEmail,
} = options
const { state, longUrl, file, description, contactEmail } = options

const url = await this.userRepository.findOneUrlForUser(userId, shortUrl)

Expand All @@ -103,7 +96,7 @@ export class UrlManagementService implements UrlManagementServiceInterface {

return this.urlRepository.update(
url,
{ longUrl, state, isSearchable, description, contactEmail },
{ longUrl, state, description, contactEmail },
storableFile,
)
}
Expand Down
Loading