Skip to content

HTM-1438: Delete scheduled task by setting search index schedule to no schedule #1044

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

HTM-1438: Delete scheduled task by setting search index schedule to no schedule #1044

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ab59390
Delete scheduled task by setting search index schedule to no schedule
steinkobben Oct 29, 2025
60cfff5
add null check for task
steinkobben Oct 31, 2025
84284e2
Merge branch 'main' into dev/htm-1438
steinkobben Oct 31, 2025
a23ae36
Do not create a new schedule if cron expression is empty
steinkobben Oct 31, 2025
e974af9
remove old hint
steinkobben Oct 31, 2025
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
3 changes: 3 additions & 0 deletions projects/admin-core/src/lib/search-index/search-index-edit/search-index-edit.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { FeatureSourceService } from '../../catalog/services/feature-source.serv
import { SearchIndexService } from '../services/search-index.service';
import { ConfirmDialogService } from '@tailormap-viewer/shared';
import { ExtendedFeatureTypeModel } from '../../catalog/models/extended-feature-type.model';
import { loadTasks } from '../../tasks/state/tasks.actions';

@Component({
selector: 'tm-admin-search-index-edit',
Expand Down Expand Up @@ -47,6 +48,8 @@ export class SearchIndexEditComponent implements OnInit {
public indexing$ = this.indexingSubject.asObservable();

public ngOnInit() {
this.store$.dispatch(loadTasks());

this.route.paramMap.pipe(
takeUntilDestroyed(this.destroyRef),
map(params => params.get('searchIndexId')),
Expand Down
3 changes: 1 addition & 2 deletions ...-core/src/lib/search-index/search-index-scheduling/search-index-scheduling.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ <h2 class="schedule-title" i18n="@@admin-core.search-index.schedule">Schedule</h
<mat-select formControlName="cronExpression">
@for (option of scheduleOptions; track $index) {
<mat-option
[value]="option.cronExpression"
[disabled]="option.cronExpression === '' && taskSchedule?.uuid">
[value]="option.cronExpression">
{{ option.viewValue }}
</mat-option>
}
Expand Down
15 changes: 6 additions & 9 deletions ...in-core/src/lib/search-index/search-index-scheduling/search-index-scheduling.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,12 @@ export class SearchIndexSchedulingComponent implements OnInit {
filter(() => this.isValidForm()),
)
.subscribe(value => {
let schedule: TaskSchedule | undefined = undefined;
if (value.cronExpression) {
schedule = {
...this.taskSchedule,
cronExpression: value.cronExpression,
description: value.description,
priority: value.priority,
};
}
const schedule = {
...this.taskSchedule,
cronExpression: value.cronExpression || '',
description: value.description,
priority: value.priority,
};
Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The schedule object is now always created even when cronExpression is empty. This changes the behavior from emitting undefined to emitting a schedule object with an empty cronExpression. This could cause issues in the parent component logic that checks for schedule existence. Consider preserving the original behavior where schedule is set to undefined when cronExpression is empty.

Suggested change
const schedule = {
...this.taskSchedule,
cronExpression: value.cronExpression || '',
description: value.description,
priority: value.priority,
};
let schedule: TaskSchedule | undefined;
if (value.cronExpression && value.cronExpression.trim() !== '') {
schedule = {
...this.taskSchedule,
cronExpression: value.cronExpression,
description: value.description,
priority: value.priority,
};
} else {
schedule = undefined;
}

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@mprins mprins Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@steinkobben this seems a valid concern, see also: #1044 (comment)

Copy link
Contributor Author

@steinkobben steinkobben Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added a check that prevents creating new schedules if the cron expression is empty. When there is an existing schedule, the schedule should be emitted with an empty cron expression because the parent component needs the schedule uuid to delete the task.

const searchIndex: Pick<SearchIndexModel, 'schedule'> = {
schedule: schedule,
};
Expand Down
28 changes: 25 additions & 3 deletions projects/admin-core/src/lib/search-index/services/search-index.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ import { AdminSnackbarService } from '../../shared/services/admin-snackbar.servi
import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
import { DebounceHelper } from '@tailormap-viewer/shared';
import { addSearchIndex, deleteSearchIndex, updateSearchIndex } from '../state/search-index.actions';
import { catchError, concatMap, map, of } from 'rxjs';
import { catchError, concatMap, map, Observable, of, switchMap, take } from 'rxjs';
import { selectDraftSearchIndex } from '../state/search-index.selectors';
import { selectTask } from '../../tasks/state/tasks.selectors';
import { TaskMonitoringService } from '../../tasks/services/task-monitoring.service';

@Injectable({
providedIn: 'root',
Expand All @@ -18,6 +20,7 @@ export class SearchIndexService {
private adminSnackbarService = inject(AdminSnackbarService);
private sseService = inject(AdminSseService);
private destroyRef = inject(DestroyRef);
private taskMonitoringService = inject(TaskMonitoringService);


public listenForSearchIndexChanges() {
Expand All @@ -42,7 +45,7 @@ export class SearchIndexService {
takeUntilDestroyed(this.destroyRef),
concatMap(searchIndex => {
if (searchIndex) {
return this.updateSearchIndex$(searchIndex.id, searchIndex);
return this.updateSearchIndexAndTask$(searchIndex.id, searchIndex);
}
return of(null);
}),
Expand All @@ -67,7 +70,26 @@ export class SearchIndexService {
);
}

public updateSearchIndex$(id: number, searchIndex: Partial<SearchIndexModel>) {
public updateSearchIndexAndTask$(id: number, searchIndex: Partial<SearchIndexModel>): Observable<SearchIndexModel | null> {
// If the schedule is being removed, delete the associated task.
// The search index should only be updated after the task is successfully deleted.
if (!searchIndex.schedule || searchIndex.schedule.cronExpression || !searchIndex.schedule.uuid) {
return this.updateSearchIndex$(id, searchIndex);
}
return this.store$.select(selectTask(searchIndex.schedule.uuid)).pipe(
take(1),
switchMap(task => this.taskMonitoringService.deleteTask$(task.uuid, task.type)),
switchMap((response) => {
if (response) {
const searchIndexEmptySchedule: Partial<SearchIndexModel> = { ...searchIndex, schedule: undefined };
return this.updateSearchIndex$(id, searchIndexEmptySchedule);
}
return of(null);
}),
);
}

public updateSearchIndex$(id: number, searchIndex: Partial<SearchIndexModel>): Observable<SearchIndexModel | null> {
return this.adminApiService.updateSearchIndex$({ id, searchIndex })
.pipe(
catchError(() => {
Expand Down
6 changes: 4 additions & 2 deletions projects/admin-core/src/lib/tasks/services/task-monitoring.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import { TailormapAdminApiV1Service } from '@tailormap-admin/admin-api';
import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
import { AdminSnackbarService } from '../../shared/services/admin-snackbar.service';

@Injectable()
@Injectable({
providedIn: 'root',
})
export class TaskMonitoringService {
private store$ = inject(Store);
private adminApiService = inject(TailormapAdminApiV1Service);
Expand Down Expand Up @@ -64,7 +66,7 @@ export class TaskMonitoringService {
this.adminApiService.stopTask$(this.uuid$.value, this.type$.value).subscribe();
}

public deleteTask(uuid: string, type: string ) {
public deleteTask$(uuid: string, type: string ) {
return this.adminApiService.deleteTask$(uuid, type)
.pipe(
catchError(() => {
Expand Down
5 changes: 2 additions & 3 deletions projects/admin-core/src/lib/tasks/task-details/task-details.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ import { reloadSearchIndexes } from '../../search-index/state/search-index.actio
selector: 'tm-admin-task-details',
templateUrl: './task-details.component.html',
styleUrls: ['./task-details.component.css'],
providers: [TaskMonitoringService],
changeDetection: ChangeDetectionStrategy.OnPush,
changeDetection: ChangeDetectionStrategy.OnPush,
standalone: false,
})
export class TaskDetailsComponent implements OnInit, OnDestroy {
Expand Down Expand Up @@ -69,7 +68,7 @@ export class TaskDetailsComponent implements OnInit, OnDestroy {
.pipe(
take(1),
filter(answer => answer),
switchMap(() => this.taskMonitoringService.deleteTask(task.uuid, task.type)),
switchMap(() => this.taskMonitoringService.deleteTask$(task.uuid, task.type)),
)
.subscribe((response) => {
if (response) {
Expand Down
1 change: 0 additions & 1 deletion projects/shared/src/lib/components/slider/slider.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ export class SliderComponent implements ControlValueAccessor {

@Input()
public set betweenValues(betweenValues: {lower: number | null; upper: number | null}) {
console.debug("Setting between values", betweenValues);
this.lowerValue = betweenValues.lower ?? this.min;
this.upperValue = betweenValues.upper ?? this.max;
}
Expand Down
Loading