Merge pull request #3791 from omnivore-app/fix/labels

fix/labels
This commit is contained in:
Hongbo Wu
2024-04-05 17:23:55 +08:00
committed by GitHub
5 changed files with 109 additions and 82 deletions

View File

@ -82,7 +82,8 @@ export const labelRepository = appDataSource.getRepository(Label).extend({
},
deleteById(id: string) {
return this.delete({ id, internal: false })
// internal labels can be deleted but it will be recreated when next feed/newsletter saved
return this.delete({ id })
},
updateLabel(id: string, label: QueryDeepPartialEntity<Label>) {

View File

@ -89,30 +89,30 @@ export const createLabelResolver = authorized<
CreateLabelSuccess,
CreateLabelError,
MutationCreateLabelArgs
>(async (_, { input }, { authTrx, log, uid }) => {
try {
const label = await authTrx(async (tx) => {
return tx.withRepository(labelRepository).createLabel(input, uid)
})
analytics.capture({
distinctId: uid,
event: 'label_created',
properties: {
...input,
env: env.server.apiEnv,
},
})
>(async (_, { input }, { authTrx, uid }) => {
const existingLabel = await labelRepository.findByName(input.name)
if (existingLabel) {
return {
label,
}
} catch (error) {
log.error('createLabelResolver', error)
return {
errorCodes: [CreateLabelErrorCode.BadRequest],
errorCodes: [CreateLabelErrorCode.LabelAlreadyExists],
}
}
const label = await authTrx(async (tx) =>
tx.withRepository(labelRepository).createLabel(input, uid)
)
analytics.capture({
distinctId: uid,
event: 'label_created',
properties: {
...input,
env: env.server.apiEnv,
},
})
return {
label,
}
})
export const deleteLabelResolver = authorized<

View File

@ -41,21 +41,18 @@ export const findOrCreateLabels = async (
labels: CreateLabelInput[],
userId: string
): Promise<Label[]> => {
// create labels if not exist
await authTrx(
async (tx) =>
tx.withRepository(labelRepository).createLabels(labels, userId),
undefined,
userId
)
// find labels
return authTrx(
async (tx) =>
tx.withRepository(labelRepository).findByNames(
labels.map((l) => l.name),
userId
),
async (tx) => {
const repo = tx.withRepository(labelRepository)
// create labels if not exist
await repo.createLabels(labels, userId)
// find labels by names
return repo.findBy({
name: In(labels.map((l) => l.name)),
user: { id: userId },
})
},
undefined,
userId
)
@ -151,6 +148,7 @@ export const saveLabelsInLibraryItem = async (
{
id: libraryItemId,
labels: labels.map((l) => deepDelete(l, columnsToDelete)),
labelNames: labels.map((l) => l.name),
},
userId
)

View File

@ -35,7 +35,7 @@ describe('Labels API', () => {
const res = await request
.post('/local/debug/fake-user-login')
.send({ fakeEmail: user.email })
authToken = res.body.authToken
authToken = res.body.authToken as string
})
after(async () => {
@ -109,18 +109,9 @@ describe('Labels API', () => {
})
describe('Create label', () => {
let query: string
let name: string
beforeEach(() => {
query = `
mutation {
createLabel(
input: {
color: "#ffffff"
name: "${name}"
}
) {
const query = `
mutation CreateLabel($input: CreateLabelInput!) {
createLabel(input: $input) {
... on CreateLabelSuccess {
label {
id
@ -133,12 +124,9 @@ describe('Labels API', () => {
}
}
`
})
context('when name not exists', () => {
before(() => {
name = 'label3'
})
const name = 'label3'
after(async () => {
// clean up
@ -146,7 +134,9 @@ describe('Labels API', () => {
})
it('should create label', async () => {
const res = await graphqlRequest(query, authToken).expect(200)
const res = await graphqlRequest(query, authToken, {
input: { name },
}).expect(200)
const label = await findLabelById(
res.body.data.createLabel.label.id,
user.id
@ -160,17 +150,30 @@ describe('Labels API', () => {
before(async () => {
existingLabel = await createLabel('label3', '#ffffff', user.id)
name = existingLabel.name
})
after(async () => {
await deleteLabels({ id: existingLabel.id }, user.id)
})
it('should return error code BAD_REQUEST', async () => {
const res = await graphqlRequest(query, authToken).expect(200)
it('should return error code LABEL_ALREADY_EXISTS', async () => {
const res = await graphqlRequest(query, authToken, {
input: { name: existingLabel.name },
}).expect(200)
expect(res.body.data.createLabel.errorCodes).to.eql(['BAD_REQUEST'])
expect(res.body.data.createLabel.errorCodes).to.eql([
'LABEL_ALREADY_EXISTS',
])
})
it('returns error code LABEL_ALREADY_EXISTS if case-insensitive label name exists', async () => {
const name = existingLabel.name.toUpperCase()
const res = await graphqlRequest(query, authToken, {
input: { name },
}).expect(200)
expect(res.body.data.createLabel.errorCodes).to.eql([
'LABEL_ALREADY_EXISTS',
])
})
})
@ -190,13 +193,11 @@ describe('Labels API', () => {
})
describe('Delete label', () => {
let query: string
let labelId: string
beforeEach(() => {
query = `
mutation {
deleteLabel(id: "${labelId}") {
const query = `
mutation DeleteLabel($labelId: ID!){
deleteLabel(id: $labelId) {
... on DeleteLabelSuccess {
label {
id
@ -209,11 +210,26 @@ describe('Labels API', () => {
}
}
`
})
context('when label exists', () => {
let toDeleteLabel: Label
context('when label is internal', () => {
before(async () => {
toDeleteLabel = await createLabel('rss', '#ffffff', user.id)
labelId = toDeleteLabel.id
})
it('should delete label', async () => {
await graphqlRequest(query, authToken, {
labelId,
}).expect(200)
const label = await findLabelById(labelId, user.id)
expect(label).not.exist
})
})
context('when label is not used', () => {
before(async () => {
toDeleteLabel = await createLabel(
@ -225,7 +241,9 @@ describe('Labels API', () => {
})
it('should delete label', async () => {
await graphqlRequest(query, authToken).expect(200)
await graphqlRequest(query, authToken, {
labelId,
}).expect(200)
const label = await findLabelById(labelId, user.id)
expect(label).not.exist
})
@ -245,7 +263,9 @@ describe('Labels API', () => {
})
it('should update page', async () => {
await graphqlRequest(query, authToken).expect(200)
await graphqlRequest(query, authToken, {
labelId,
}).expect(200)
const updatedItem = await findLibraryItemById(item.id, user.id)
expect(updatedItem?.labels).not.deep.include(toDeleteLabel)
@ -280,7 +300,9 @@ describe('Labels API', () => {
})
it('should update highlight', async () => {
await graphqlRequest(query, authToken).expect(200)
await graphqlRequest(query, authToken, {
labelId,
}).expect(200)
const updatedHighlight = await findHighlightById(highlightId, user.id)
expect(updatedHighlight?.labels).not.deep.include(toDeleteLabel)
@ -294,7 +316,9 @@ describe('Labels API', () => {
})
it('should return error code NOT_FOUND', async () => {
const res = await graphqlRequest(query, authToken).expect(200)
const res = await graphqlRequest(query, authToken, {
labelId,
}).expect(200)
expect(res.body.data.deleteLabel.errorCodes).to.eql(['NOT_FOUND'])
})
@ -334,10 +358,7 @@ describe('Labels API', () => {
after(async () => {
// clean up
await deleteLabels(
{ user: { id: user.id } },
user.id
)
await deleteLabels({ user: { id: user.id } }, user.id)
await deleteLibraryItemById(item.id)
})
@ -464,7 +485,7 @@ describe('Labels API', () => {
})
after(async () => {
await deleteLabels( { id: toUpdateLabel.id }, user.id)
await deleteLabels({ id: toUpdateLabel.id }, user.id)
})
it('should return the updated label', async () => {
@ -538,10 +559,7 @@ describe('Labels API', () => {
after(async () => {
// clean up
await deleteLabels(
{ user: { id: user.id } },
user.id
)
await deleteLabels({ user: { id: user.id } }, user.id)
await deleteLibraryItemById(item.id)
})
@ -588,7 +606,9 @@ describe('Labels API', () => {
it('should set labels for highlight', async () => {
const res = await graphqlRequest(query, authToken).expect(200)
expect(
res.body.data.setLabelsForHighlight.labels.map((l: any) => l.id)
(res.body.data.setLabelsForHighlight.labels as Label[]).map(
(l) => l.id
)
).to.eql(labelIds)
})
})
@ -652,7 +672,7 @@ describe('Labels API', () => {
`
let labelId: string
let afterLabelId: string
let labels: Label[] = []
const labels: Label[] = []
before(async () => {
// create testing labels
@ -671,7 +691,7 @@ describe('Labels API', () => {
})
context('when label exists', () => {
before(async () => {
before(() => {
labelId = labels[1].id
afterLabelId = labels[4].id
})

View File

@ -25,6 +25,14 @@ type CreateRuleModalProps = {
revalidate: () => void
}
const eventTypeObj = {
PAGE_CREATED: 'PAGE_CREATED',
PAGE_UPDATED: 'PAGE_UPDATED',
HIGHLIGHT_CREATED: 'HIGHLIGHT_CREATED',
HIGHLIGHT_UPDATED: 'HIGHLIGHT_UPDATED',
LABEL_CREATED: 'LABEL_ATTACHED',
}
const CreateRuleModal = (props: CreateRuleModalProps): JSX.Element => {
const [form] = Form.useForm()
@ -104,7 +112,7 @@ const CreateRuleModal = (props: CreateRuleModalProps): JSX.Element => {
const value = Object.values(RuleEventType)[index]
return (
<Select.Option key={key} value={value}>
{key === 'LABEL_CREATED' ? 'LABEL_ATTACHED' : key}
{eventTypeObj[value]}
</Select.Option>
)
})}
@ -363,7 +371,7 @@ export default function Rules(): JSX.Element {
{row.eventTypes.map((eventType: RuleEventType, index: number) => {
return (
<Tag color={'geekblue'} key={index}>
{eventType}
{eventTypeObj[eventType]}
</Tag>
)
})}