Change tags to use uids instead of ids. (#351)

* Small cleanups

* Change tags to use uid instead of id.

---------

Co-authored-by: antanst <>
This commit is contained in:
Antonis 2025-09-22 17:10:29 +03:00 committed by GitHub
parent 8b01e456b3
commit 70956f9ecd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
17 changed files with 97 additions and 133 deletions

View file

@ -2,7 +2,7 @@ const express = require('express');
const { Note, Tag, Project, sequelize } = require('../models');
const { Op } = require('sequelize');
const { extractUidFromSlug } = require('../utils/slug-utils');
const { validateTagName } = require('../utils/validation');
const { validateTagName } = require('../services/tagsService');
const router = express.Router();
// Helper function to update note tags

View file

@ -7,7 +7,7 @@ const fs = require('fs');
const { Project, Task, Tag, Area, Note, sequelize } = require('../models');
const { Op } = require('sequelize');
const { extractUidFromSlug } = require('../utils/slug-utils');
const { validateTagName } = require('../utils/validation');
const { validateTagName } = require('../services/tagsService');
const { uid } = require('../utils/uid');
const router = express.Router();

View file

@ -1,16 +1,17 @@
const express = require('express');
const { Tag, Task, Note, Project, sequelize } = require('../models');
const { extractUidFromSlug } = require('../utils/slug-utils');
const { validateTagName } = require('../utils/validation');
const { validateTagName } = require('../services/tagsService');
const router = express.Router();
const _ = require('lodash');
const { Op } = require('sequelize');
// GET /api/tags
router.get('/tags', async (req, res) => {
try {
const tags = await Tag.findAll({
where: { user_id: req.currentUser.id },
attributes: ['id', 'name', 'uid'],
attributes: ['name', 'uid'],
order: [['name', 'ASC']],
});
res.json(tags);
@ -20,17 +21,14 @@ router.get('/tags', async (req, res) => {
}
});
// GET /api/tag/:identifier (supports both ID, name, and uid-slug)
// GET /api/tag/:identifier (supports name and uid)
router.get('/tag', async (req, res) => {
try {
const { id, uid, name } = req.query;
const { uid, name } = req.query;
let whereClause = {
user_id: req.currentUser.id,
};
if (!_.isEmpty(id)) {
whereClause.id = parseInt(id, 10);
}
if (!_.isEmpty(uid)) {
whereClause.uid = uid;
}
@ -43,7 +41,7 @@ router.get('/tag', async (req, res) => {
attributes: ['name', 'uid'],
});
if (!tag) {
if (_.isEmpty(tag)) {
return res.status(404).json({ error: 'Tag not found' });
}
@ -70,8 +68,7 @@ router.post('/tag', async (req, res) => {
});
res.status(201).json({
id: tag.id,
uid: tag.uid, // Explicitly include uid
uid: tag.uid,
name: tag.name,
});
} catch (error) {
@ -85,21 +82,13 @@ router.post('/tag', async (req, res) => {
// PATCH /api/tag/:identifier (supports both ID and name)
router.patch('/tag/:identifier', async (req, res) => {
try {
const identifier = req.params.identifier;
let whereClause;
// Check if identifier is a number (ID) or string (name)
if (/^\d+$/.test(identifier)) {
// It's a numeric ID
whereClause = {
id: parseInt(identifier),
user_id: req.currentUser.id,
};
} else {
// It's a tag name - decode URI component to handle special characters
const tagName = decodeURIComponent(identifier);
whereClause = { name: tagName, user_id: req.currentUser.id };
}
const param = decodeURIComponent(req.params.identifier);
let whereClause = {
[Op.or]: [
{ name: param, user_id: req.currentUser.id },
{ uid: param, user_id: req.currentUser.id },
],
};
const tag = await Tag.findOne({
where: whereClause,
@ -110,7 +99,6 @@ router.patch('/tag/:identifier', async (req, res) => {
}
const { name } = req.body;
const validation = validateTagName(name);
if (!validation.valid) {
return res.status(400).json({ error: validation.error });
@ -130,79 +118,51 @@ router.patch('/tag/:identifier', async (req, res) => {
}
});
// DELETE /api/tag/:identifier (supports both ID and name)
// DELETE /api/tag/:identifier (supports uid and name)
router.delete('/tag/:identifier', async (req, res) => {
const transaction = await sequelize.transaction();
try {
const identifier = req.params.identifier;
let whereClause;
// Check if identifier is a number (ID) or string (name)
if (/^\d+$/.test(identifier)) {
// It's a numeric ID
whereClause = {
id: parseInt(identifier),
user_id: req.currentUser.id,
};
} else {
// It's a tag name - decode URI component to handle special characters
const tagName = decodeURIComponent(identifier);
whereClause = { name: tagName, user_id: req.currentUser.id };
}
const param = decodeURIComponent(req.params.identifier);
let whereClause = {
[Op.or]: [
{ name: param, user_id: req.currentUser.id },
{ uid: param, user_id: req.currentUser.id },
],
};
const tag = await Tag.findOne({
where: whereClause,
});
if (!tag) {
if (_.isEmpty(tag)) {
await transaction.rollback();
return res.status(404).json({ error: 'Tag not found' });
}
// Use transaction to ensure all deletions happen atomically
// Remove all associations before deleting the tag by manually deleting from junction tables
// Only delete from tables that exist
try {
await sequelize.query('DELETE FROM tasks_tags WHERE tag_id = ?', {
await Promise.all([
sequelize.query('DELETE FROM tasks_tags WHERE tag_id = ?', {
replacements: [tag.id],
type: sequelize.QueryTypes.DELETE,
transaction,
});
} catch (error) {
// Ignore if table doesn't exist
console.log('tasks_tags table not found, skipping');
}
try {
await sequelize.query('DELETE FROM notes_tags WHERE tag_id = ?', {
}),
sequelize.query('DELETE FROM notes_tags WHERE tag_id = ?', {
replacements: [tag.id],
type: sequelize.QueryTypes.DELETE,
transaction,
});
} catch (error) {
// Ignore if table doesn't exist
console.log('notes_tags table not found, skipping');
}
}),
sequelize.query('DELETE FROM projects_tags WHERE tag_id = ?', {
replacements: [tag.id],
type: sequelize.QueryTypes.DELETE,
transaction,
}),
]);
try {
await sequelize.query(
'DELETE FROM projects_tags WHERE tag_id = ?',
{
replacements: [tag.id],
type: sequelize.QueryTypes.DELETE,
transaction,
}
);
} catch (error) {
// Ignore if table doesn't exist
console.log('projects_tags table not found, skipping');
}
// Now safely delete the tag
await tag.destroy({ transaction });
await transaction.commit();
res.json({ message: 'Tag successfully deleted' });
} catch (error) {
await transaction.rollback();

View file

@ -18,7 +18,7 @@ const {
logTaskUpdate,
getTaskTodayMoveCount,
} = require('../services/taskEventService');
const { validateTagName } = require('../utils/validation');
const { validateTagName } = require('../services/tagsService');
const {
getSafeTimezone,
getUpcomingRangeInUTC,

View file

@ -1,11 +1,5 @@
const { TaskEvent } = require('../models');
// Helper function to add default source to metadata
const addDefaultSource = (metadata) => ({
source: 'web',
...metadata,
});
// Helper function to create value object
const createValueObject = (fieldName, value) =>
value ? { [fieldName || 'value']: value } : null;
@ -31,7 +25,10 @@ const logEvent = async ({
metadata = {},
}) => {
try {
const finalMetadata = addDefaultSource(metadata);
const finalMetadata = {
source: 'web',
...metadata,
};
const event = await TaskEvent.create({
task_id: taskId,

View file

@ -29,7 +29,7 @@ describe('Tags Routes', () => {
expect(response.status).toBe(201);
expect(response.body.name).toBe(tagData.name);
expect(response.body.id).toBeDefined();
expect(response.body.uid).toBeDefined();
});
it('should require authentication', async () => {
@ -61,7 +61,7 @@ describe('Tags Routes', () => {
expect(response.status).toBe(201);
expect(response.body.name).toBe('project:frontend');
expect(response.body.id).toBeDefined();
expect(response.body.uid).toBeDefined();
});
it('should allow hyphen (-) in tag names', async () => {
@ -73,7 +73,7 @@ describe('Tags Routes', () => {
expect(response.status).toBe(201);
expect(response.body.name).toBe('project-frontend');
expect(response.body.id).toBeDefined();
expect(response.body.uid).toBeDefined();
});
it('should reject tags with invalid characters', async () => {
@ -108,8 +108,8 @@ describe('Tags Routes', () => {
expect(response.status).toBe(200);
expect(response.body).toHaveLength(2);
expect(response.body.map((t) => t.id)).toContain(tag1.id);
expect(response.body.map((t) => t.id)).toContain(tag2.id);
expect(response.body.map((t) => t.uid)).toContain(tag1.uid);
expect(response.body.map((t) => t.uid)).toContain(tag2.uid);
});
it('should order tags by name', async () => {
@ -138,15 +138,15 @@ describe('Tags Routes', () => {
});
});
it('should get tag by id', async () => {
const response = await agent.get(`/api/tag?id=${tag.id}`);
it('should get tag by uid', async () => {
const response = await agent.get(`/api/tag?uid=${tag.uid}`);
expect(response.status).toBe(200);
expect(response.body.name).toBe(tag.name);
});
it('should return 404 for non-existent tag', async () => {
const response = await agent.get('/api/tag?id=999999');
const response = await agent.get('/api/tag?uid=non-existent-uid');
expect(response.status).toBe(404);
expect(response.body.error).toBe('Tag not found');
@ -164,14 +164,14 @@ describe('Tags Routes', () => {
user_id: otherUser.id,
});
const response = await agent.get(`/api/tag?id=${otherTag.id}`);
const response = await agent.get(`/api/tag?uid=${otherTag.uid}`);
expect(response.status).toBe(404);
expect(response.body.error).toBe('Tag not found');
});
it('should require authentication', async () => {
const response = await request(app).get(`/api/tag?id=${tag.id}`);
const response = await request(app).get(`/api/tag?uid=${tag.uid}`);
expect(response.status).toBe(401);
expect(response.body.error).toBe('Authentication required');
@ -194,7 +194,7 @@ describe('Tags Routes', () => {
};
const response = await agent
.patch(`/api/tag/${tag.id}`)
.patch(`/api/tag/${tag.uid}`)
.send(updateData);
expect(response.status).toBe(200);
@ -203,7 +203,7 @@ describe('Tags Routes', () => {
it('should return 404 for non-existent tag', async () => {
const response = await agent
.patch('/api/tag/999999')
.patch('/api/tag/non-existent-uid')
.send({ name: 'Updated' });
expect(response.status).toBe(404);
@ -223,7 +223,7 @@ describe('Tags Routes', () => {
});
const response = await agent
.patch(`/api/tag/${otherTag.id}`)
.patch(`/api/tag/${otherTag.uid}`)
.send({ name: 'Updated' });
expect(response.status).toBe(404);
@ -232,7 +232,7 @@ describe('Tags Routes', () => {
it('should require authentication', async () => {
const response = await request(app)
.patch(`/api/tag/${tag.id}`)
.patch(`/api/tag/${tag.uid}`)
.send({ name: 'Updated' });
expect(response.status).toBe(401);
@ -251,7 +251,7 @@ describe('Tags Routes', () => {
});
it('should delete tag', async () => {
const response = await agent.delete(`/api/tag/${tag.id}`);
const response = await agent.delete(`/api/tag/${tag.uid}`);
expect(response.status).toBe(200);
expect(response.body.message).toBe('Tag successfully deleted');
@ -262,7 +262,7 @@ describe('Tags Routes', () => {
});
it('should return 404 for non-existent tag', async () => {
const response = await agent.delete('/api/tag/999999');
const response = await agent.delete('/api/tag/non-existent-uid');
expect(response.status).toBe(404);
expect(response.body.error).toBe('Tag not found');
@ -280,14 +280,14 @@ describe('Tags Routes', () => {
user_id: otherUser.id,
});
const response = await agent.delete(`/api/tag/${otherTag.id}`);
const response = await agent.delete(`/api/tag/${otherTag.uid}`);
expect(response.status).toBe(404);
expect(response.body.error).toBe('Tag not found');
});
it('should require authentication', async () => {
const response = await request(app).delete(`/api/tag/${tag.id}`);
const response = await request(app).delete(`/api/tag/${tag.uid}`);
expect(response.status).toBe(401);
expect(response.body.error).toBe('Authentication required');

View file

@ -1,4 +1,4 @@
const { validateTagName } = require('../../../utils/validation');
const { validateTagName } = require('../../../services/tagsService');
describe('validation utils', () => {
describe('validateTagName', () => {

View file

@ -288,15 +288,15 @@ const Layout: React.FC<LayoutProps> = ({
const handleSaveTag = async (tagData: Tag) => {
try {
let result: Tag;
if (tagData.id) {
result = await updateTag(tagData.id, tagData);
if (tagData.uid) {
result = await updateTag(tagData.uid, tagData);
// Update existing tag in global store
const currentTags = useStore.getState().tagsStore.tags;
useStore
.getState()
.tagsStore.setTags(
currentTags.map((tag) =>
tag.id === result.id ? result : tag
tag.uid === result.uid ? result : tag
)
);
} else {

View file

@ -1644,7 +1644,11 @@ const InboxModal: React.FC<InboxModalProps> = ({
>
{filteredTags.map((tag, index) => (
<button
key={tag.id || index}
key={
tag.uid ||
tag.id ||
index
}
onClick={() =>
handleTagSelect(
tag.name

View file

@ -694,7 +694,7 @@ const ProjectDetails: React.FC = () => {
<TagIcon className="h-3 w-3 text-white/70 flex-shrink-0 mt-0.5" />
<div className="flex items-center space-x-1">
{project.tags.map((tag, index) => (
<span key={tag.id || index}>
<span key={tag.uid || tag.id || index}>
<button
onClick={() => {
// Navigate to tag details page

View file

@ -248,7 +248,7 @@ const TagInput: React.FC<TagInputProps> = ({
>
{filteredTags.map((tag, index) => (
<button
key={tag.id}
key={tag.uid || tag.id}
type="button"
onClick={() => selectTag(tag.name)}
className={`w-full text-left px-4 py-2 text-sm hover:bg-gray-200 dark:hover:bg-gray-700 ${

View file

@ -8,7 +8,7 @@ interface TagModalProps {
isOpen: boolean;
onClose: () => void;
onSave: (tag: Tag) => void;
onDelete?: (tagId: number) => void;
onDelete?: (tagUid: string) => void;
tag?: Tag | null;
}
@ -127,9 +127,9 @@ const TagModal: React.FC<TagModalProps> = ({
};
const handleDeleteTag = async () => {
if (formData.id && onDelete) {
if (formData.uid && onDelete) {
try {
await onDelete(formData.id);
await onDelete(formData.uid);
showSuccessToast(
t('success.tagDeleted', 'Tag deleted successfully!')
);
@ -193,7 +193,7 @@ const TagModal: React.FC<TagModalProps> = ({
<div className="flex-shrink-0 bg-white dark:bg-gray-800 border-t border-gray-200 dark:border-gray-700 px-3 py-2 flex items-center justify-between sm:rounded-b-lg">
{/* Left side: Delete and Cancel */}
<div className="flex items-center space-x-3">
{tag && tag.id && onDelete && (
{tag && tag.uid && onDelete && (
<button
type="button"
onClick={handleDeleteTag}

View file

@ -133,8 +133,8 @@ const Tags: React.FC = () => {
const handleDeleteTag = async () => {
if (!tagToDelete) return;
try {
await apiDeleteTag(tagToDelete.id!);
setTags(tags.filter((tag) => tag.id !== tagToDelete.id));
await apiDeleteTag(tagToDelete.uid!);
setTags(tags.filter((tag) => tag.uid !== tagToDelete.uid));
// Remove the deleted tag from metrics as well
setTagMetrics((prev) => {
const newMetrics = { ...prev };
@ -155,10 +155,10 @@ const Tags: React.FC = () => {
const handleSaveTag = async (tagData: Tag) => {
try {
if (tagData.id) {
await updateTag(tagData.id, tagData);
if (tagData.uid) {
await updateTag(tagData.uid, tagData);
setTags(
tags.map((tag) => (tag.id === tagData.id ? tagData : tag))
tags.map((tag) => (tag.uid === tagData.uid ? tagData : tag))
);
} else {
const newTag = await createTag(tagData);
@ -281,7 +281,7 @@ const Tags: React.FC = () => {
return (
<li
key={tag.id}
key={tag.uid || tag.id}
className="bg-white dark:bg-gray-900 shadow rounded-lg p-4"
onMouseEnter={() =>
setHoveredTagId(
@ -372,7 +372,7 @@ const Tags: React.FC = () => {
className={`text-gray-500 hover:text-blue-700 dark:hover:text-blue-300 focus:outline-none transition-opacity ${hoveredTagId === tag.id ? 'opacity-100' : 'opacity-0'}`}
aria-label={`Edit ${tag.name}`}
title={`Edit ${tag.name}`}
data-testid={`tag-edit-${tag.id}`}
data-testid={`tag-edit-${tag.uid || tag.id}`}
>
<PencilSquareIcon className="h-4 w-4" />
</button>
@ -385,7 +385,7 @@ const Tags: React.FC = () => {
className={`text-gray-500 hover:text-red-700 dark:hover:text-red-300 focus:outline-none transition-opacity ${hoveredTagId === tag.id ? 'opacity-100' : 'opacity-0'}`}
aria-label={`Delete ${tag.name}`}
title={`Delete ${tag.name}`}
data-testid={`tag-delete-${tag.id}`}
data-testid={`tag-delete-${tag.uid || tag.id}`}
>
<TrashIcon className="h-4 w-4" />
</button>
@ -409,14 +409,16 @@ const Tags: React.FC = () => {
setSelectedTag(null);
}}
onSave={handleSaveTag}
onDelete={async (tagId) => {
onDelete={async (tagUid) => {
try {
await apiDeleteTag(tagId);
setTags(tags.filter((tag) => tag.id !== tagId));
await apiDeleteTag(tagUid);
setTags(
tags.filter((tag) => tag.uid !== tagUid)
);
setTagMetrics((prev) => {
const newMetrics = { ...prev };
const deletedTag = tags.find(
(t) => t.id === tagId
(t) => t.uid === tagUid
);
if (deletedTag) {
delete newMetrics[deletedTag.name];

View file

@ -484,6 +484,7 @@ const TaskDetails: React.FC = () => {
) => (
<React.Fragment
key={
tag.uid ||
tag.id ||
tag.name
}

View file

@ -5,7 +5,7 @@ import { TagIcon, XMarkIcon } from '@heroicons/react/24/solid';
interface TaskTagsProps {
tags: Tag[];
onTagRemove?: (tagId: string | number | undefined) => void;
onTagRemove?: (tagUid: string | undefined) => void;
className?: string;
}
@ -38,7 +38,7 @@ const TaskTags: React.FC<TaskTagsProps> = ({
<div className={`flex flex-wrap gap-2 ${className}`}>
{tags.map((tag, index) => (
<div
key={tag.id || index}
key={tag.uid || tag.id || index}
className="flex items-center bg-gray-200 text-gray-800 text-xs font-medium px-2 py-1.5 rounded-md dark:bg-gray-700 dark:text-gray-200 cursor-pointer"
>
<button
@ -54,7 +54,7 @@ const TaskTags: React.FC<TaskTagsProps> = ({
{onTagRemove && (
<button
type="button"
onClick={() => onTagRemove(tag.id)}
onClick={() => onTagRemove(tag.uid)}
className="ml-1 text-gray-500 hover:text-gray-700 dark:hover:text-gray-400 focus:outline-none"
aria-label={`Remove tag ${tag.name}`}
>

View file

@ -35,8 +35,8 @@ export const createTag = async (tagData: Tag): Promise<Tag> => {
return await response.json();
};
export const updateTag = async (tagId: number, tagData: Tag): Promise<Tag> => {
const response = await fetch(`/api/tag/${tagId}`, {
export const updateTag = async (tagUid: string, tagData: Tag): Promise<Tag> => {
const response = await fetch(`/api/tag/${tagUid}`, {
method: 'PATCH',
credentials: 'include',
headers: {
@ -50,8 +50,8 @@ export const updateTag = async (tagId: number, tagData: Tag): Promise<Tag> => {
return await response.json();
};
export const deleteTag = async (tagId: number): Promise<void> => {
const response = await fetch(`/api/tag/${tagId}`, {
export const deleteTag = async (tagUid: string): Promise<void> => {
const response = await fetch(`/api/tag/${tagUid}`, {
method: 'DELETE',
credentials: 'include',
headers: {