Skip to content

Per saving or viewing tags fails #564

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

Merged
merged 2 commits into from
May 14, 2025
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
2 changes: 1 addition & 1 deletion src/app/core/services/tags/tags.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ describe('TagsService', () => {

it('should only cache tags from the current archive', () => {
const item = new RecordVO({
TagVOs: [
tags: [
{ tagId: 1, name: 'testOne', archiveId: 1 },
{ tagId: 2, name: 'testTwo', archiveId: 2 },
],
Expand Down
42 changes: 31 additions & 11 deletions src/app/core/services/tags/tags.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import { Injectable } from '@angular/core';
import { TagVOData } from '@models/tag-vo';
import debug from 'debug';
import { ItemVO } from '@models';
import { FolderVO, ItemVO, RecordVO } from '@models';
import { AccountService } from '@shared/services/account/account.service';
import { orderBy, find } from 'lodash';
import { Subject } from 'rxjs';
Expand Down Expand Up @@ -53,21 +53,41 @@
}

checkTagsOnItem(item: ItemVO) {
if (!item.TagVOs?.length) {
if (
(item.isRecord && !item.tags?.length) ||
(item.isFolder && !item.TagVOs?.length)
) {
return;
}

let hasNew = false;

for (const itemTag of item.TagVOs) {
if (
!this.tags.has(itemTag.tagId) &&
itemTag.name &&
itemTag.archiveId === this.account.getArchive().archiveId
) {
this.tags.set(itemTag.tagId, itemTag);
hasNew = true;
this.debug('new tag seen %o', itemTag);
if (item.isFolder) {
for (const itemTag of item.TagVOs) {

Check warning on line 66 in src/app/core/services/tags/tags.service.ts

View check run for this annotation

Codecov / codecov/patch

src/app/core/services/tags/tags.service.ts#L66

Added line #L66 was not covered by tests
if (
!this.tags.has(itemTag.tagId) &&
itemTag.name &&
itemTag.archiveId === this.account.getArchive().archiveId
) {
this.tags.set(itemTag.tagId, itemTag);
hasNew = true;
this.debug('new tag seen %o', itemTag);

Check warning on line 74 in src/app/core/services/tags/tags.service.ts

View check run for this annotation

Codecov / codecov/patch

src/app/core/services/tags/tags.service.ts#L72-L74

Added lines #L72 - L74 were not covered by tests
}
}
}

if (item.isRecord) {
for (const itemTag of (item as RecordVO).tags) {
if (
!this.tags.has(itemTag.id) &&
itemTag.name &&
itemTag.archiveId === this.account.getArchive().archiveId
) {
const tagId = itemTag.tagId ?? itemTag.id;
this.tags.set(tagId, { ...itemTag, tagId });
hasNew = true;
this.debug('new tag seen %o', itemTag);
}
}
}

Expand Down
142 changes: 78 additions & 64 deletions src/app/file-browser/components/edit-tags/edit-tags.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { async } from '@angular/core/testing';
import { Shallow } from 'shallow-render';
import { Observable, of } from 'rxjs';

import { ItemVO, TagVOData, RecordVO } from '@models';
import { ItemVO, TagVOData, RecordVO, FolderVO } from '@models';
import { ApiService } from '@shared/services/api/api.service';
import { DataService } from '@shared/services/data/data.service';
import { TagsService } from '@core/services/tags/tags.service';
Expand Down Expand Up @@ -38,12 +38,35 @@ const defaultTagList: TagVOData[] = [
type: 'type.tag.metadata.customField',
},
];
const defaultItem: ItemVO = new RecordVO({ TagVOs: defaultTagList });
const defaultRecord = new RecordVO({
tags: defaultTagList,
isRecord: true,
});

const defaultFolder = new FolderVO({
TagVOs: defaultTagList,
isFolder: true,
});

describe('EditTagsComponent', () => {
let shallow: Shallow<EditTagsComponent>;
async function defaultRender(
item: ItemVO = defaultItem,
async function defaultRenderRecord(
item: ItemVO = defaultRecord,
tagType: TagType = 'keyword',
) {
return await shallow.render(
`<pr-edit-tags [item]="item" [tagType]="tagType"></pr-edit-tags>`,
{
bind: {
item: item,
tagType: tagType,
},
},
);
}

async function defaultRenderFolder(
item: ItemVO = defaultFolder,
tagType: TagType = 'keyword',
) {
return await shallow.render(
Expand Down Expand Up @@ -81,14 +104,32 @@ describe('EditTagsComponent', () => {
);
}));

it('should create', async () => {
const { element } = await defaultRender();
it('should create record tags', async () => {
const { element } = await defaultRenderRecord();

expect(element).not.toBeNull();
});

it('should only show keywords in keyword mode', async () => {
const { element } = await defaultRender();
it('should create folder tags', async () => {
const { element } = await defaultRenderFolder();

expect(element).not.toBeNull();
});

it('should only show keywords in keyword mode for records', async () => {
const { element, fixture } = await defaultRenderRecord();

element.componentInstance.itemTags = [
{ name: 'tagOne' },
{ name: 'tagTwo' },
];

element.componentInstance.matchingTags = [
{ name: 'tagOne' },
{ name: 'tagTwo' },
];

fixture.detectChanges();

expect(
element.componentInstance.itemTags.find((tag) => tag.name === 'tagOne'),
Expand Down Expand Up @@ -135,65 +176,20 @@ describe('EditTagsComponent', () => {
).not.toBeTruthy();
});

it('should only show custom metadata in custom metadata mode', async () => {
const { element } = await defaultRender(defaultItem, 'customMetadata');

expect(
element.componentInstance.itemTags.find((tag) => tag.name === 'tagOne'),
).not.toBeTruthy();

expect(
element.componentInstance.itemTags.find((tag) => tag.name === 'tagTwo'),
).not.toBeTruthy();

expect(
element.componentInstance.itemTags.find(
(tag) => tag.name === 'customField:customValueOne',
),
).toBeTruthy();

expect(
element.componentInstance.itemTags.find(
(tag) => tag.name === 'customField:customValueTwo',
),
).toBeTruthy();

expect(
element.componentInstance.matchingTags.find(
(tag) => tag.name === 'tagOne',
),
).not.toBeTruthy();

expect(
element.componentInstance.matchingTags.find(
(tag) => tag.name === 'tagTwo',
),
).not.toBeTruthy();

expect(
element.componentInstance.matchingTags.find(
(tag) => tag.name === 'customField:customValueOne',
),
).toBeTruthy();

expect(
element.componentInstance.matchingTags.find(
(tag) => tag.name === 'customField:customValueTwo',
),
).toBeTruthy();
});

it('should not create custom metadata in keyword mode', async () => {
const { element } = await defaultRender();
it('should not create custom metadata in keyword mode for records', async () => {
const { element } = await defaultRenderRecord();
const tagCreateSpy = spyOn(element.componentInstance.api.tag, 'create');
await element.componentInstance.onInputEnter('key:value');

expect(element.componentInstance.newTagInputError).toBeTruthy();
expect(tagCreateSpy).not.toHaveBeenCalled();
});

it('should not create keyword in custom metadata mode', async () => {
const { element } = await defaultRender(defaultItem, 'customMetadata');
it('should not create keyword in custom metadata mode for records', async () => {
const { element } = await defaultRenderRecord(
defaultRecord,
'customMetadata',
);
const tagCreateSpy = spyOn(element.componentInstance.api.tag, 'create');
await element.componentInstance.onInputEnter('keyword');

Expand All @@ -202,7 +198,25 @@ describe('EditTagsComponent', () => {
});

it('should highlight the correct tag on key down', async () => {
const { fixture, element } = await defaultRender();
const { fixture, element } = await defaultRenderFolder();

element.componentInstance.isEditing = true;

fixture.detectChanges();
const tags = fixture.debugElement.queryAll(By.css('.edit-tag'));

const arrowKeyDown = new KeyboardEvent('keydown', { key: 'ArrowDown' });
tags[0].nativeElement.dispatchEvent(arrowKeyDown);

fixture.detectChanges();

const focusedElement = document.activeElement as HTMLElement;

expect(focusedElement).toBe(tags[1].nativeElement);
});

it('should highlight the correct tag for folders on key down', async () => {
const { fixture, element } = await defaultRenderFolder();

element.componentInstance.isEditing = true;

Expand All @@ -220,7 +234,7 @@ describe('EditTagsComponent', () => {
});

it('should highlight the correct tag on key up', async () => {
const { fixture, element } = await defaultRender();
const { fixture, element } = await defaultRenderRecord();

element.componentInstance.isEditing = true;

Expand All @@ -238,7 +252,7 @@ describe('EditTagsComponent', () => {
});

it('should highlight the input on key up', async () => {
const { fixture, element } = await defaultRender();
const { fixture, element } = await defaultRenderRecord();

element.componentInstance.isEditing = true;

Expand All @@ -258,7 +272,7 @@ describe('EditTagsComponent', () => {
});

it('should open dialog when manage link is clicked', async () => {
const { element, find, inject, fixture } = await defaultRender();
const { element, find, inject, fixture } = await defaultRenderRecord();
const dialogOpenSpy = inject(DialogCdkService);

element.componentInstance.isEditing = true;
Expand Down
66 changes: 46 additions & 20 deletions src/app/file-browser/components/edit-tags/edit-tags.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
Inject,
} from '@angular/core';
import { TagsService } from '@core/services/tags/tags.service';
import { ItemVO, TagVOData, TagLinkVOData, FolderVO } from '@models';
import { ItemVO, TagVOData, TagLinkVOData, FolderVO, RecordVO } from '@models';
import { DataService } from '@shared/services/data/data.service';
import { Subject, Subscription } from 'rxjs';
import {
Expand Down Expand Up @@ -81,9 +81,9 @@
private tagsService: TagsService,
private message: MessageService,
private api: ApiService,
private dataService: DataService,
private elementRef: ElementRef,
private dialog: DialogCdkService,
private dataService: DataService,
) {
this.subscriptions.push(
this.tagsService.getTags$().subscribe((tags) => {
Expand Down Expand Up @@ -113,9 +113,15 @@
(tag) => !tag.type.includes('type.tag.metadata'),
);
} else {
this.dialogTags = tags?.filter((tag) =>
tag.type.includes('type.tag.metadata'),
);
this.dialogTags = tags
?.filter((tag) => tag.type.includes('type.tag.metadata'))
.map((tag) => ({

Check warning on line 118 in src/app/file-browser/components/edit-tags/edit-tags.component.ts

View check run for this annotation

Codecov / codecov/patch

src/app/file-browser/components/edit-tags/edit-tags.component.ts#L116-L118

Added lines #L116 - L118 were not covered by tests
id: tag.id,
name: `${tag.name}:${
tag.type.split('.')[tag.type.split.length - 1]
}`,
type: tag.type,
}));
}
});
}
Expand Down Expand Up @@ -162,7 +168,7 @@
this.onTagType(this.newTagName);
}

async onTagClick(tag: TagVOData) {
async onTagClick(tag) {
const tagLink: TagLinkVOData = {};
if (this.item instanceof FolderVO) {
tagLink.refTable = 'folder';
Expand All @@ -174,10 +180,14 @@

this.waiting = true;
try {
if (tag.tagId && this.itemTagsById.has(tag.tagId)) {
if (
(tag.tagId && this.itemTagsById.has(tag.tagId)) ||
(tag.id && this.itemTagsById.has(tag.id))
) {
await this.api.tag.deleteTagLink(tag, tagLink);
} else {
await this.api.tag.create(tag, tagLink);
await this.tagsService.refreshTags();

Check warning on line 190 in src/app/file-browser/components/edit-tags/edit-tags.component.ts

View check run for this annotation

Codecov / codecov/patch

src/app/file-browser/components/edit-tags/edit-tags.component.ts#L190

Added line #L190 was not covered by tests
}
await this.dataService.fetchFullItems([this.item]);
} catch (err) {
Expand Down Expand Up @@ -232,23 +242,39 @@

this.itemTagsById.clear();

this.itemTags = this.filterTagsByType(
(this.item?.TagVOs || [])
.map((tag) => this.allTags?.find((t) => t.tagId === tag.tagId))
.filter(
// Filter out tags that are now null from deletion
(tag) => tag?.name,
),
);
if (this.item && this.item?.isFolder) {
this.itemTags = this.filterTagsByType(
(this.item?.TagVOs || [])
.map((tag) => this.allTags?.find((t) => t.tagId === tag.tagId))
.filter(
// Filter out tags that are now null from deletion
(tag) => tag?.name,
),
);
}

if (!this.item?.TagVOs?.length) {
return;
if (this.item && this.item?.isRecord) {
this.itemTags = this.filterTagsByType(
(this.item?.tags || [])
.map((tag) => this.allTags?.find((t) => t.tagId === +tag.id))
.filter(
// Filter out tags that are now null from deletion
(tag) => tag?.name,
),
);
}

if (Array.isArray(this.itemTags)) {
for (const tag of this.itemTags) {
this.itemTagsById.add(tag.tagId);
}
}

for (const tag of this.itemTags) {
this.itemTagsById.add(tag.tagId);
if (this.item) {
this.tagsService.setItemTags(
this.item.isFolder ? this.item.TagVOs : (this.item as RecordVO).tags,
);
}
this.tagsService.setItemTags(this.item.TagVOs);
}

onManageTagsClick() {
Expand Down
Loading