feat(releases): delete based on age/indexer/status (#1522)

* feat(releases): delete based on age/indexer/status

* fix: sanitize releaseStatuses

* swap to RMSC

* add AgeSelect component

* improve texts

* refactor: streamline form layout

* improve text

* remove a paragraph

* improved UX

explaining the options, better error handling

* reinstate red border

* fix: labels to match other similar labels for selects

- improved contrast for the word "required" in desc
- added red asterisk to required select

* minor text improvement to warning

* fix: delete-button vertical alignment

* feat: cleanup queries

* feat: cleanup delete

---------

Co-authored-by: ze0s <ze0s@riseup.net>

internal/database/release.go

@@ -582,7 +582,27 @@ func (repo *ReleaseRepo) Delete(ctx context.Context, req *domain.DeleteReleaseRe
 		return errors.Wrap(err, "could not start transaction")
 	}
 
-	defer tx.Rollback()
+	defer func() {
+		var txErr error
+		if p := recover(); p != nil {
+			txErr = tx.Rollback()
+			if txErr != nil {
+				repo.log.Error().Err(txErr).Msg("error rolling back transaction")
+			}
+			repo.log.Error().Msgf("something went terribly wrong panic: %v", p)
+		} else if err != nil {
+			txErr = tx.Rollback()
+			if txErr != nil {
+				repo.log.Error().Err(txErr).Msg("error rolling back transaction")
+			}
+		} else {
+			// All good, commit
+			txErr = tx.Commit()
+			if txErr != nil {
+				repo.log.Error().Err(txErr).Msg("error committing transaction")
+			}
+		}
+	}()
 
 	qb := repo.db.squirrel.Delete("release")
 
@@ -599,16 +619,30 @@ func (repo *ReleaseRepo) Delete(ctx context.Context, req *domain.DeleteReleaseRe
 		}
 	}
 
-	query, args, err := qb.ToSql()
+	if len(req.Indexers) > 0 {
-	if err != nil {
+		qb = qb.Where(sq.Eq{"indexer": req.Indexers})
-		return errors.Wrap(err, "error executing query")
 	}
 
-	repo.log.Debug().Str("repo", "release").Str("query", query).Msgf("release.delete: args: %v", args)
+	if len(req.ReleaseStatuses) > 0 {
+		subQuery := sq.Select("release_id").From("release_action_status").Where(sq.Eq{"status": req.ReleaseStatuses})
+		subQueryText, subQueryArgs, err := subQuery.ToSql()
+		if err != nil {
+			return errors.Wrap(err, "error building subquery")
+		}
+		qb = qb.Where("id IN ("+subQueryText+")", subQueryArgs...)
+	}
+
+	query, args, err := qb.ToSql()
+	if err != nil {
+		return errors.Wrap(err, "error building SQL query")
+	}
+
+	repo.log.Trace().Str("query", query).Interface("args", args).Msg("Executing combined delete query")
 
 	result, err := tx.ExecContext(ctx, query, args...)
 	if err != nil {
-		return errors.Wrap(err, "error executing query")
+		repo.log.Error().Err(err).Str("query", query).Interface("args", args).Msg("Error executing combined delete query")
+		return errors.Wrap(err, "error executing delete query")
 	}
 
 	deletedRows, err := result.RowsAffected()
@@ -616,16 +650,20 @@ func (repo *ReleaseRepo) Delete(ctx context.Context, req *domain.DeleteReleaseRe
 		return errors.Wrap(err, "error fetching rows affected")
 	}
 
-	_, err = tx.ExecContext(ctx, `DELETE FROM release_action_status WHERE release_id NOT IN (SELECT id FROM "release")`)
+	repo.log.Debug().Msgf("deleted %d rows from release table", deletedRows)
+
+	// clean up orphaned rows
+	orphanedResult, err := tx.ExecContext(ctx, `DELETE FROM release_action_status WHERE release_id NOT IN (SELECT id FROM "release")`)
 	if err != nil {
 		return errors.Wrap(err, "error executing query")
 	}
 
-	if err := tx.Commit(); err != nil {
+	deletedRowsOrphaned, err := orphanedResult.RowsAffected()
-		return errors.Wrap(err, "error commit transaction delete")
+	if err != nil {
+		return errors.Wrap(err, "error fetching rows affected")
 	}
 
-	repo.log.Debug().Msgf("deleted %d rows from release table", deletedRows)
+	repo.log.Debug().Msgf("deleted %d orphaned rows from release table", deletedRowsOrphaned)
 
 	return nil
 }

internal/domain/release.go

@@ -124,7 +124,9 @@ type ReleaseActionStatus struct {
 }
 
 type DeleteReleaseRequest struct {
-	OlderThan int
+	OlderThan       int
+	Indexers        []string
+	ReleaseStatuses []string
 }
 
 func NewReleaseActionStatus(action *Action, release *Release) *ReleaseActionStatus {

internal/http/release.go

@@ -211,6 +211,31 @@ func (h releaseHandler) deleteReleases(w http.ResponseWriter, r *http.Request) {
 		req.OlderThan = duration
 	}
 
+	indexers := r.URL.Query()["indexer"]
+	if len(indexers) > 0 {
+		req.Indexers = indexers
+	}
+
+	releaseStatuses := r.URL.Query()["releaseStatus"]
+	validStatuses := map[string]bool{
+		"PUSH_APPROVED": true,
+		"PUSH_REJECTED": true,
+		"PUSH_ERROR":    true,
+	}
+	var filteredStatuses []string
+	for _, status := range releaseStatuses {
+		if _, valid := validStatuses[status]; valid {
+			filteredStatuses = append(filteredStatuses, status)
+		} else {
+			h.encoder.StatusResponse(w, http.StatusBadRequest, map[string]interface{}{
+				"code":    "INVALID_RELEASE_STATUS",
+				"message": "releaseStatus contains invalid value",
+			})
+			return
+		}
+	}
+	req.ReleaseStatuses = filteredStatuses
+
 	if err := h.service.Delete(r.Context(), &req); err != nil {
 		h.encoder.Error(w, err)
 		return

web/src/api/APIClient.ts

@@ -15,6 +15,14 @@ interface HttpConfig {
   queryString?: Record<string, Primitive | Primitive[]>;
 }
 
+interface DeleteParams {
+  olderThan?: number;
+  indexers?: string[];
+  releaseStatuses?: string[];
+}
+
+type QueryStringParams = Record<string, string | string[]>;
+
 // See https://stackoverflow.com/a/62969380
 function encodeRFC3986URIComponent(str: string): string {
   return encodeURIComponent(str).replace(
@@ -338,9 +346,22 @@ export const APIClient = {
     },
     indexerOptions: () => appClient.Get<string[]>("api/release/indexers"),
     stats: () => appClient.Get<ReleaseStats>("api/release/stats"),
-    delete: (olderThan: number) => appClient.Delete("api/release", {
+    delete: (params: DeleteParams) => {
-      queryString: { olderThan }
+      const queryString: QueryStringParams = {};
-    }),
+      if (params.olderThan !== undefined) {
+        queryString.olderThan = params.olderThan.toString();
+      }
+      if (params.indexers && params.indexers.length > 0) {
+        queryString.indexer = params.indexers;
+      }
+      if (params.releaseStatuses && params.releaseStatuses.length > 0) {
+        queryString.releaseStatus = params.releaseStatuses;
+      }
+    
+      return appClient.Delete("api/release", {
+        queryString
+      });
+    },
     replayAction: (releaseId: number, actionId: number) => appClient.Post(
       `api/release/${releaseId}/actions/${actionId}/retry`
     )

web/src/components/inputs/select.tsx

@@ -476,3 +476,85 @@ export const SelectWide = ({
     </div>
   );
 };
+
+export const AgeSelect = ({
+  duration,
+  setDuration,
+  setParsedDuration,
+  columns = 6
+}: {
+  duration: string;
+  setDuration: (value: string) => void;
+  setParsedDuration: (value: number) => void;
+  columns?: number;
+}) => {
+  const options = [
+    { value: '1', label: '1 hour' },
+    { value: '12', label: '12 hours' },
+    { value: '24', label: '1 day' },
+    { value: '168', label: '1 week' },
+    { value: '720', label: '1 month' },
+    { value: '2160', label: '3 months' },
+    { value: '4320', label: '6 months' },
+    { value: '8760', label: '1 year' },
+    { value: '0', label: 'Delete everything' }
+  ];
+
+  return (
+    <div className={`col-span-12 ${columns ? `sm:col-span-${columns}` : ""}`}>
+      <Listbox value={duration} onChange={(value) => {
+        const parsedValue = parseInt(value, 10);
+        setParsedDuration(parsedValue);
+        setDuration(value);
+      }}>
+        {({ open }) => (
+          <>
+            <div className="mt-0 relative">
+              <Listbox.Button className="block w-full relative shadow-sm text-sm text-left rounded-md border pl-3 pr-10 py-2.5 focus:ring-blue-500 dark:focus:ring-blue-500 focus:border-blue-500 dark:focus:border-blue-500 border-gray-300 dark:border-gray-700 bg-gray-100 dark:bg-gray-815 dark:text-gray-400">
+                <span className="block truncate text-gray-500 dark:text-white">
+                  {duration ? options.find(opt => opt.value === duration)?.label : 'Select...'}
+                </span>
+                <span className="absolute inset-y-0 right-0 flex items-center pr-2 pointer-events-none">
+                  <ChevronUpDownIcon className="h-5 w-5 text-gray-700 dark:text-gray-500" aria-hidden="true" />
+                </span>
+              </Listbox.Button>
+              <Transition
+                show={open}
+                as={Fragment}
+                leave="transition ease-in duration-100"
+                leaveFrom="opacity-100"
+                leaveTo="opacity-0"
+              >
+                <Listbox.Options className="absolute z-10 mt-1 w-full shadow-lg max-h-60 rounded-md py-1 overflow-auto border border-gray-300 dark:border-gray-700 bg-gray-100 dark:bg-gray-815 dark:text-white focus:outline-none text-sm">
+                  {options.map((option) => (
+                    <Listbox.Option
+                      key={option.value}
+                      className={({ active, selected }) =>
+                        `relative cursor-default select-none py-2 pl-3 pr-9 ${selected ? "font-bold text-black dark:text-white bg-gray-300 dark:bg-gray-950" : active ? "text-black dark:text-gray-100 font-normal bg-gray-200 dark:bg-gray-800" : "text-gray-700 dark:text-gray-300 font-normal"
+                        }`
+                      }
+                      value={option.value}
+                    >
+                      {({ selected }) => (
+                        <>
+                          <span className="block truncate">{option.label}</span>
+                          {selected && (
+                            <span className="absolute inset-y-0 right-0 flex items-center pr-4">
+                              <CheckIcon className="h-5 w-5 text-blue-600 dark:text-blue-500" aria-hidden="true" />
+                            </span>
+                          )}
+                        </>
+                      )}
+                    </Listbox.Option>
+                  ))}
+                </Listbox.Options>
+              </Transition>
+            </div>
+          </>
+        )}
+      </Listbox>
+    </div>
+  );
+};
+
+

web/src/screens/settings/Releases.tsx

@@ -4,8 +4,10 @@
  */
 
 import { useRef, useState } from "react";
-import { useMutation, useQueryClient } from "@tanstack/react-query";
+import { useMutation, useQueryClient, useQuery } from "@tanstack/react-query";
 import { toast } from "react-hot-toast";
+import { MultiSelect as RMSC } from "react-multi-select-component";
+import { AgeSelect } from "@components/inputs"
 
 import { APIClient } from "@api/APIClient";
 import { ReleaseKeys } from "@api/query_keys";
@@ -20,19 +22,11 @@ const ReleaseSettings = () => (
     description="Manage release history."
   >
     <div className="border border-red-500 rounded">
-      <div className="py-6 px-4 sm:p-6">
-        <div>
-          <h2 className="text-lg leading-4 font-bold text-gray-900 dark:text-white">Danger zone</h2>
-          <p className="mt-1 text-sm text-gray-500 dark:text-gray-400">
-            This will clear release history in your database
-          </p>
-        </div>
-      </div>
-
       <div className="py-6 px-4 sm:p-6">
         <DeleteReleases />
       </div>
     </div>
+
   </Section>
 );
 
@@ -53,38 +47,65 @@ const getDurationLabel = (durationValue: number): string => {
   return durationOptions[durationValue] || "Invalid duration";
 };
 
+interface Indexer {
+  label: string;
+  value: string;
+}
+
+interface ReleaseStatus {
+  label: string;
+  value: string;
+}
+
 function DeleteReleases() {
   const queryClient = useQueryClient();
   const [duration, setDuration] = useState<string>("");
-  const [parsedDuration, setParsedDuration] = useState<number>(0);
+  const [parsedDuration, setParsedDuration] = useState<number>();
+  const [indexers, setIndexers] = useState<Indexer[]>([]);
+  const [releaseStatuses, setReleaseStatuses] = useState<ReleaseStatus[]>([]);
   const cancelModalButtonRef = useRef<HTMLInputElement | null>(null);
   const [deleteModalIsOpen, toggleDeleteModal] = useToggle(false);
 
+  const { data: indexerOptions } = useQuery<IndexerDefinition[], Error, { identifier: string; name: string; }[]>({
+    queryKey: ['indexers'],
+    queryFn: () => APIClient.indexers.getAll(),
+    select: data => data.map(indexer => ({
+      identifier: indexer.identifier,
+      name: indexer.name
+    })),
+  });
+
+  const releaseStatusOptions = [
+    { label: "Approved", value: "PUSH_APPROVED" },
+    { label: "Rejected", value: "PUSH_REJECTED" },
+    { label: "Errored", value: "PUSH_ERROR" }
+  ];
+
   const deleteOlderMutation = useMutation({
-    mutationFn: (olderThan: number) => APIClient.release.delete(olderThan),
+    mutationFn: (params: { olderThan: number, indexers: string[], releaseStatuses: string[] }) =>
+      APIClient.release.delete(params),
     onSuccess: () => {
       if (parsedDuration === 0) {
         toast.custom((t) => (
-          <Toast type="success" body={"All releases were deleted."} t={t} />
+          <Toast type="success" body={"All releases based on criteria were deleted."} t={t} />
         ));
       } else {
         toast.custom((t) => (
-          <Toast type="success" body={`Releases older than ${getDurationLabel(parsedDuration)} were deleted.`} t={t} />
+          <Toast type="success" body={`Releases older than ${getDurationLabel(parsedDuration ?? 0)} were deleted.`} t={t} />
         ));
       }
 
-      // Invalidate filters just in case, most likely not necessary but can't hurt.
       queryClient.invalidateQueries({ queryKey: ReleaseKeys.lists() });
     }
   });
 
   const deleteOlderReleases = () => {
-    if (isNaN(parsedDuration) || parsedDuration < 0) {
+    if (parsedDuration === undefined || isNaN(parsedDuration) || parsedDuration < 0) {
-      toast.custom((t) => <Toast type="error" body={"Please select a valid duration."} t={t} />);
+      toast.custom((t) => <Toast type="error" body={"Please select a valid age."} t={t} />);
       return;
     }
 
-    deleteOlderMutation.mutate(parsedDuration);
+    deleteOlderMutation.mutate({ olderThan: parsedDuration, indexers: indexers.map(i => i.value), releaseStatuses: releaseStatuses.map(rs => rs.value) });
   };
 
   return (
@@ -96,46 +117,79 @@ function DeleteReleases() {
         buttonRef={cancelModalButtonRef}
         deleteAction={deleteOlderReleases}
         title="Remove releases"
-        text={`Are you sure you want to remove releases older than ${getDurationLabel(parsedDuration)}? This action cannot be undone.`}
+        text={`You are about to ${parsedDuration ? `permanently delete all release history records older than ${getDurationLabel(parsedDuration)} for ` : 'delete all release history records for '}${indexers.length ? 'the chosen indexers' : 'all indexers'}${releaseStatuses.length ? ` and with the following release statuses: ${releaseStatuses.map(status => status.label).join(', ')}` : ''}.`}
       />
+      <div className="flex flex-col gap-2 w-full">
+        <div>
+          <h2 className="text-lg leading-4 font-bold text-gray-900 dark:text-white">Delete release history</h2>
+          <p className="text-sm mt-1 text-gray-500 dark:text-gray-400">
+            Select the criteria below to permanently delete release history records that are older than the chosen age and optionally match the selected indexers and release statuses:
+            <ul className="list-disc pl-5 mt-2">
+              <li>
+                Older than (e.g., 6 months - all records older than 6 months will be deleted) - <strong className="text-gray-600 dark:text-gray-300">Required</strong>
+              </li>
+              <li>Indexers - Optional (if none selected, applies to all indexers)</li>
+              <li>Release statuses - Optional (if none selected, applies to all release statuses)</li>
+            </ul>
+            <p className="mt-2 text-red-600 dark:text-red-500">
+              <strong>Warning:</strong> If no indexers or release statuses are selected, all release history records older than the selected age will be permanently deleted, regardless of indexer or status.
+            </p>
+          </p>
+        </div>
 
-      <label htmlFor="duration" className="flex flex-col">
+        <div className="flex flex-col sm:flex-row gap-2 pt-4 items-center text-sm">
-        <p className="text-sm font-medium text-gray-900 dark:text-white">Delete</p>
+          {[
-        <p className="text-sm text-gray-500 dark:text-gray-400">Delete releases older than select duration</p>
+            {
-      </label>
+              label: (
-      <div className="flex flex-wrap gap-2">
+                <>
-        <select
+                  Older than:
-          name="duration"
+                  <span className="text-red-600 dark:text-red-500"> *</span>
-          id="duration"
+                </>
-          className="focus:outline-none focus:ring-1 focus:ring-offset-0 focus:ring-blue-500 dark:focus:ring-blue-500 rounded-md sm:text-sm border-gray-300 dark:border-gray-600 dark:bg-gray-800 dark:text-white"
+              ),
-          value={duration}
+              content: <AgeSelect duration={duration} setDuration={setDuration} setParsedDuration={setParsedDuration} />
-          onChange={(e) => {
+            },
-            const parsedDuration = parseInt(e.target.value, 10);
+            {
-            setParsedDuration(parsedDuration);
+              label: 'Indexers:',
-            setDuration(e.target.value);
+              content: <RMSC options={indexerOptions?.map(option => ({ value: option.identifier, label: option.name })) || []} value={indexers} onChange={setIndexers} labelledBy="Select indexers" />
-          }}
+            },
-        >
+            {
-          <option value="">Select duration</option>
+              label: 'Release statuses:',
-          <option value="1">1 hour</option>
+              content: <RMSC options={releaseStatusOptions} value={releaseStatuses} onChange={setReleaseStatuses} labelledBy="Select release statuses" />
-          <option value="12">12 hours</option>
+            }
-          <option value="24">1 day</option>
+          ].map((item, index) => (
-          <option value="168">1 week</option>
+            <div key={index} className="flex flex-col w-full">
-          <option value="720">1 month</option>
+              <p className="text-xs font-bold text-gray-800 dark:text-gray-100 uppercase p-1 cursor-default">{item.label}</p>
-          <option value="2160">3 months</option>
+              {item.content}
-          <option value="4320">6 months</option>
+            </div>
-          <option value="8760">1 year</option>
+          ))}
-          <option value="0">Delete everything</option>
+          <button
-        </select>
+            type="button"
-        <button
+            onClick={() => {
-          type="button"
+              if (parsedDuration === undefined || isNaN(parsedDuration)) {
-          onClick={toggleDeleteModal}
+                toast.custom((t) => (
-          className="inline-flex items-center px-4 py-2 border border-transparent text-sm font-medium rounded-md text-red-700 hover:text-red-800 dark:text-white bg-red-200 dark:bg-red-700 hover:bg-red-300 dark:hover:bg-red-800 focus:outline-none focus:ring-1 focus:ring-inset focus:ring-red-600"
+                  <Toast
-        >
+                    type="error"
-          Delete
+                    body={
-        </button>
+                      "Please enter a valid age. For example, 6 months or 1 year."
+                    }
+                    t={t}
+                  />
+                ));
+              } else {
+                toggleDeleteModal();
+              }
+            }}
+            className="inline-flex justify-center sm:w-1/5 md:w-1/5 w-full px-4 py-2 sm:mt-6 border border-transparent text-sm font-medium rounded-md text-red-700 hover:text-red-800 dark:text-white bg-red-200 dark:bg-red-700 hover:bg-red-300 dark:hover:bg-red-800 focus:outline-none focus:ring-1 focus:ring-inset focus:ring-red-600"
+          >
+            Delete
+          </button>
+
+        </div>
       </div>
     </div>
+
   );
+
 }
 
 export default ReleaseSettings;