Skip to content

Commit

Permalink
feat(releases): delete based on age/indexer/status (#1522)
Browse files Browse the repository at this point in the history 
* 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>
  • Loading branch information
@s0up4200 @zze0s
s0up4200 and zze0s committed May 3, 2024
1 parent f8715c1 commit 19e129e
Show file tree
Hide file tree
Showing 6 changed files with 287 additions and 65 deletions.
54 changes: 46 additions & 8 deletions internal/database/release.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand All @@ -599,33 +619,51 @@ func (repo *ReleaseRepo) Delete(ctx context.Context, req *domain.DeleteReleaseRe
}
}

if len(req.Indexers) > 0 {
qb = qb.Where(sq.Eq{"indexer": req.Indexers})
}

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 executing query")
return errors.Wrap(err, "error building SQL query")
}

repo.log.Debug().Str("repo", "release").Str("query", query).Msgf("release.delete: args: %v", args)
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()
if err != nil {
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 {
return errors.Wrap(err, "error commit transaction delete")
deletedRowsOrphaned, err := orphanedResult.RowsAffected()
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
}
Expand Down
4 changes: 3 additions & 1 deletion internal/domain/release.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
25 changes: 25 additions & 0 deletions internal/http/release.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 24 additions & 3 deletions web/src/api/APIClient.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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", {
queryString: { olderThan }
}),
delete: (params: DeleteParams) => {
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`
)
Expand Down
82 changes: 82 additions & 0 deletions web/src/components/inputs/select.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
);
};


0 comments on commit 19e129e

Please sign in to comment.