From 8e1947d9712e23ba92a67bb613fd5b5e892367fe Mon Sep 17 00:00:00 2001 From: Hayden <64056131+hay-kot@users.noreply.github.com> Date: Wed, 2 Nov 2022 11:54:43 -0800 Subject: [PATCH] fix: conditionally filter parent locations (#133) --- backend/app/api/handlers/v1/query_params.go | 36 +++++++++++++++++ backend/app/api/handlers/v1/v1_ctrl_items.go | 40 +++---------------- .../app/api/handlers/v1/v1_ctrl_locations.go | 10 ++++- backend/app/api/static/docs/docs.go | 8 ++++ backend/app/api/static/docs/swagger.json | 8 ++++ backend/app/api/static/docs/swagger.yaml | 5 +++ .../internal/core/services/service_items.go | 16 ++++---- .../core/services/service_items_test.go | 3 +- backend/internal/data/repo/repo_locations.go | 18 +++++++-- .../internal/data/repo/repo_locations_test.go | 2 +- frontend/components/Item/CreateModal.vue | 2 +- frontend/layouts/default.vue | 6 ++- frontend/lib/api/classes/locations.ts | 8 +++- frontend/pages/home.vue | 2 +- frontend/pages/item/[id]/edit.vue | 2 +- frontend/pages/items.vue | 2 +- frontend/pages/location/[id].vue | 2 +- frontend/stores/locations.ts | 32 +++++++++++---- 18 files changed, 135 insertions(+), 67 deletions(-) create mode 100644 backend/app/api/handlers/v1/query_params.go diff --git a/backend/app/api/handlers/v1/query_params.go b/backend/app/api/handlers/v1/query_params.go new file mode 100644 index 0000000..0ac84d8 --- /dev/null +++ b/backend/app/api/handlers/v1/query_params.go @@ -0,0 +1,36 @@ +package v1 + +import ( + "net/url" + "strconv" + + "github.com/google/uuid" +) + +func queryUUIDList(params url.Values, key string) []uuid.UUID { + var ids []uuid.UUID + for _, id := range params[key] { + uid, err := uuid.Parse(id) + if err != nil { + continue + } + ids = append(ids, uid) + } + return ids +} + +func queryIntOrNegativeOne(s string) int { + i, err := strconv.Atoi(s) + if err != nil { + return -1 + } + return i +} + +func queryBool(s string) bool { + b, err := strconv.ParseBool(s) + if err != nil { + return false + } + return b +} diff --git a/backend/app/api/handlers/v1/v1_ctrl_items.go b/backend/app/api/handlers/v1/v1_ctrl_items.go index 732600f..f4d43d3 100644 --- a/backend/app/api/handlers/v1/v1_ctrl_items.go +++ b/backend/app/api/handlers/v1/v1_ctrl_items.go @@ -3,10 +3,7 @@ package v1 import ( "encoding/csv" "net/http" - "net/url" - "strconv" - "github.com/google/uuid" "github.com/hay-kot/homebox/backend/internal/core/services" "github.com/hay-kot/homebox/backend/internal/data/repo" "github.com/hay-kot/homebox/backend/internal/sys/validate" @@ -27,44 +24,17 @@ import ( // @Router /v1/items [GET] // @Security Bearer func (ctrl *V1Controller) HandleItemsGetAll() server.HandlerFunc { - uuidList := func(params url.Values, key string) []uuid.UUID { - var ids []uuid.UUID - for _, id := range params[key] { - uid, err := uuid.Parse(id) - if err != nil { - continue - } - ids = append(ids, uid) - } - return ids - } - - intOrNegativeOne := func(s string) int { - i, err := strconv.Atoi(s) - if err != nil { - return -1 - } - return i - } - - getBool := func(s string) bool { - b, err := strconv.ParseBool(s) - if err != nil { - return false - } - return b - } extractQuery := func(r *http.Request) repo.ItemQuery { params := r.URL.Query() return repo.ItemQuery{ - Page: intOrNegativeOne(params.Get("page")), - PageSize: intOrNegativeOne(params.Get("perPage")), + Page: queryIntOrNegativeOne(params.Get("page")), + PageSize: queryIntOrNegativeOne(params.Get("perPage")), Search: params.Get("q"), - LocationIDs: uuidList(params, "locations"), - LabelIDs: uuidList(params, "labels"), - IncludeArchived: getBool(params.Get("includeArchived")), + LocationIDs: queryUUIDList(params, "locations"), + LabelIDs: queryUUIDList(params, "labels"), + IncludeArchived: queryBool(params.Get("includeArchived")), } } diff --git a/backend/app/api/handlers/v1/v1_ctrl_locations.go b/backend/app/api/handlers/v1/v1_ctrl_locations.go index 5e85766..12e3c29 100644 --- a/backend/app/api/handlers/v1/v1_ctrl_locations.go +++ b/backend/app/api/handlers/v1/v1_ctrl_locations.go @@ -15,13 +15,21 @@ import ( // @Summary Get All Locations // @Tags Locations // @Produce json +// @Param filterChildren query bool false "Filter locations with parents" // @Success 200 {object} server.Results{items=[]repo.LocationOutCount} // @Router /v1/locations [GET] // @Security Bearer func (ctrl *V1Controller) HandleLocationGetAll() server.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) error { user := services.UseUserCtx(r.Context()) - locations, err := ctrl.repo.Locations.GetAll(r.Context(), user.GroupID) + + q := r.URL.Query() + + filter := repo.LocationQuery{ + FilterChildren: queryBool(q.Get("filterChildren")), + } + + locations, err := ctrl.repo.Locations.GetAll(r.Context(), user.GroupID, filter) if err != nil { log.Err(err).Msg("failed to get locations") return validate.NewRequestError(err, http.StatusInternalServerError) diff --git a/backend/app/api/static/docs/docs.go b/backend/app/api/static/docs/docs.go index a4105a1..f2c877e 100644 --- a/backend/app/api/static/docs/docs.go +++ b/backend/app/api/static/docs/docs.go @@ -756,6 +756,14 @@ const docTemplate = `{ "Locations" ], "summary": "Get All Locations", + "parameters": [ + { + "type": "boolean", + "description": "Filter locations with parents", + "name": "filterChildren", + "in": "query" + } + ], "responses": { "200": { "description": "OK", diff --git a/backend/app/api/static/docs/swagger.json b/backend/app/api/static/docs/swagger.json index 911f8f2..7215fae 100644 --- a/backend/app/api/static/docs/swagger.json +++ b/backend/app/api/static/docs/swagger.json @@ -748,6 +748,14 @@ "Locations" ], "summary": "Get All Locations", + "parameters": [ + { + "type": "boolean", + "description": "Filter locations with parents", + "name": "filterChildren", + "in": "query" + } + ], "responses": { "200": { "description": "OK", diff --git a/backend/app/api/static/docs/swagger.yaml b/backend/app/api/static/docs/swagger.yaml index b4f8c78..e479583 100644 --- a/backend/app/api/static/docs/swagger.yaml +++ b/backend/app/api/static/docs/swagger.yaml @@ -960,6 +960,11 @@ paths: - Labels /v1/locations: get: + parameters: + - description: Filter locations with parents + in: query + name: filterChildren + type: boolean produces: - application/json responses: diff --git a/backend/internal/core/services/service_items.go b/backend/internal/core/services/service_items.go index 5c02724..4af80c8 100644 --- a/backend/internal/core/services/service_items.go +++ b/backend/internal/core/services/service_items.go @@ -23,7 +23,7 @@ type ItemService struct { at attachmentTokens } -func (svc *ItemService) CsvImport(ctx context.Context, gid uuid.UUID, data [][]string) (int, error) { +func (svc *ItemService) CsvImport(ctx context.Context, GID uuid.UUID, data [][]string) (int, error) { loaded := []csvRow{} // Skip first row @@ -66,7 +66,7 @@ func (svc *ItemService) CsvImport(ctx context.Context, gid uuid.UUID, data [][]s // Bootstrap the locations and labels so we can reuse the created IDs for the items locations := map[string]uuid.UUID{} - existingLocation, err := svc.repo.Locations.GetAll(ctx, gid) + existingLocation, err := svc.repo.Locations.GetAll(ctx, GID, repo.LocationQuery{}) if err != nil { return 0, err } @@ -75,7 +75,7 @@ func (svc *ItemService) CsvImport(ctx context.Context, gid uuid.UUID, data [][]s } labels := map[string]uuid.UUID{} - existingLabels, err := svc.repo.Labels.GetAll(ctx, gid) + existingLabels, err := svc.repo.Labels.GetAll(ctx, GID) if err != nil { return 0, err } @@ -87,7 +87,7 @@ func (svc *ItemService) CsvImport(ctx context.Context, gid uuid.UUID, data [][]s // Locations if _, exists := locations[row.Location]; !exists { - result, err := svc.repo.Locations.Create(ctx, gid, repo.LocationCreate{ + result, err := svc.repo.Locations.Create(ctx, GID, repo.LocationCreate{ Name: row.Location, Description: "", }) @@ -103,7 +103,7 @@ func (svc *ItemService) CsvImport(ctx context.Context, gid uuid.UUID, data [][]s if _, exists := labels[label]; exists { continue } - result, err := svc.repo.Labels.Create(ctx, gid, repo.LabelCreate{ + result, err := svc.repo.Labels.Create(ctx, GID, repo.LabelCreate{ Name: label, Description: "", }) @@ -119,7 +119,7 @@ func (svc *ItemService) CsvImport(ctx context.Context, gid uuid.UUID, data [][]s for _, row := range loaded { // Check Import Ref if row.Item.ImportRef != "" { - exists, err := svc.repo.Items.CheckRef(ctx, gid, row.Item.ImportRef) + exists, err := svc.repo.Items.CheckRef(ctx, GID, row.Item.ImportRef) if exists { continue } @@ -139,7 +139,7 @@ func (svc *ItemService) CsvImport(ctx context.Context, gid uuid.UUID, data [][]s Str("location", row.Location). Msgf("Creating Item: %s", row.Item.Name) - result, err := svc.repo.Items.Create(ctx, gid, repo.ItemCreate{ + result, err := svc.repo.Items.Create(ctx, GID, repo.ItemCreate{ ImportRef: row.Item.ImportRef, Name: row.Item.Name, Description: row.Item.Description, @@ -152,7 +152,7 @@ func (svc *ItemService) CsvImport(ctx context.Context, gid uuid.UUID, data [][]s } // Update the item with the rest of the data - _, err = svc.repo.Items.UpdateByGroup(ctx, gid, repo.ItemUpdate{ + _, err = svc.repo.Items.UpdateByGroup(ctx, GID, repo.ItemUpdate{ // Edges LocationID: locationID, LabelIDs: labelIDs, diff --git a/backend/internal/core/services/service_items_test.go b/backend/internal/core/services/service_items_test.go index 1daa0b7..105c842 100644 --- a/backend/internal/core/services/service_items_test.go +++ b/backend/internal/core/services/service_items_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/google/uuid" + "github.com/hay-kot/homebox/backend/internal/data/repo" "github.com/stretchr/testify/assert" ) @@ -38,7 +39,7 @@ func TestItemService_CsvImport(t *testing.T) { dataCsv = append(dataCsv, newCsvRow(item)) } - allLocation, err := tRepos.Locations.GetAll(context.Background(), tGroup.ID) + allLocation, err := tRepos.Locations.GetAll(context.Background(), tGroup.ID, repo.LocationQuery{}) assert.NoError(t, err) locNames := []string{} for _, loc := range allLocation { diff --git a/backend/internal/data/repo/repo_locations.go b/backend/internal/data/repo/repo_locations.go index f7d63eb..641b3e3 100644 --- a/backend/internal/data/repo/repo_locations.go +++ b/backend/internal/data/repo/repo_locations.go @@ -2,6 +2,7 @@ package repo import ( "context" + "strings" "time" "github.com/google/uuid" @@ -90,8 +91,12 @@ func mapLocationOut(location *ent.Location) LocationOut { } } +type LocationQuery struct { + FilterChildren bool `json:"filterChildren"` +} + // GetALlWithCount returns all locations with item count field populated -func (r *LocationRepository) GetAll(ctx context.Context, groupId uuid.UUID) ([]LocationOutCount, error) { +func (r *LocationRepository) GetAll(ctx context.Context, GID uuid.UUID, filter LocationQuery) ([]LocationOutCount, error) { query := `--sql SELECT id, @@ -111,13 +116,18 @@ func (r *LocationRepository) GetAll(ctx context.Context, groupId uuid.UUID) ([]L FROM locations WHERE - locations.group_locations = ? - AND locations.location_children IS NULL + locations.group_locations = ? {{ FILTER_CHILDREN }} ORDER BY locations.name ASC ` - rows, err := r.db.Sql().QueryContext(ctx, query, groupId) + if filter.FilterChildren { + query = strings.Replace(query, "{{ FILTER_CHILDREN }}", "AND locations.location_children IS NULL", 1) + } else { + query = strings.Replace(query, "{{ FILTER_CHILDREN }}", "", 1) + } + + rows, err := r.db.Sql().QueryContext(ctx, query, GID) if err != nil { return nil, err } diff --git a/backend/internal/data/repo/repo_locations_test.go b/backend/internal/data/repo/repo_locations_test.go index d48779d..5085d3b 100644 --- a/backend/internal/data/repo/repo_locations_test.go +++ b/backend/internal/data/repo/repo_locations_test.go @@ -43,7 +43,7 @@ func TestLocationRepositoryGetAllWithCount(t *testing.T) { assert.NoError(t, err) - results, err := tRepos.Locations.GetAll(context.Background(), tGroup.ID) + results, err := tRepos.Locations.GetAll(context.Background(), tGroup.ID, LocationQuery{}) assert.NoError(t, err) for _, loc := range results { diff --git a/frontend/components/Item/CreateModal.vue b/frontend/components/Item/CreateModal.vue index 42bb6b7..2a2aec3 100644 --- a/frontend/components/Item/CreateModal.vue +++ b/frontend/components/Item/CreateModal.vue @@ -41,7 +41,7 @@ const toast = useNotifier(); const locationsStore = useLocationStore(); - const locations = computed(() => locationsStore.locations); + const locations = computed(() => locationsStore.allLocations); const labelStore = useLabelStore(); const labels = computed(() => labelStore.labels); diff --git a/frontend/layouts/default.vue b/frontend/layouts/default.vue index 5b6d488..ab21150 100644 --- a/frontend/layouts/default.vue +++ b/frontend/layouts/default.vue @@ -32,7 +32,8 @@ const rmLocationStoreObserver = defineObserver("locationStore", { handler: r => { if (r.status === 201 || r.url.match(reLocation)) { - locationStore.refresh(); + locationStore.refreshChildren(); + locationStore.refreshParents(); } console.debug("locationStore handler called by observer"); }, @@ -43,7 +44,8 @@ EventTypes.ClearStores, () => { labelStore.refresh(); - locationStore.refresh(); + locationStore.refreshChildren(); + locationStore.refreshParents(); }, "stores" ); diff --git a/frontend/lib/api/classes/locations.ts b/frontend/lib/api/classes/locations.ts index d456e91..7559c62 100644 --- a/frontend/lib/api/classes/locations.ts +++ b/frontend/lib/api/classes/locations.ts @@ -2,9 +2,13 @@ import { BaseAPI, route } from "../base"; import { LocationOutCount, LocationCreate, LocationOut, LocationUpdate } from "../types/data-contracts"; import { Results } from "../types/non-generated"; +export type LocationsQuery = { + filterChildren: boolean; +}; + export class LocationsApi extends BaseAPI { - getAll() { - return this.http.get>({ url: route("/locations") }); + getAll(q: LocationsQuery = { filterChildren: false }) { + return this.http.get>({ url: route("/locations", q) }); } create(body: LocationCreate) { diff --git a/frontend/pages/home.vue b/frontend/pages/home.vue index 1adc178..0113a0b 100644 --- a/frontend/pages/home.vue +++ b/frontend/pages/home.vue @@ -16,7 +16,7 @@ const auth = useAuthStore(); const locationStore = useLocationStore(); - const locations = computed(() => locationStore.locations); + const locations = computed(() => locationStore.parentLocations); const labelsStore = useLabelStore(); const labels = computed(() => labelsStore.labels); diff --git a/frontend/pages/item/[id]/edit.vue b/frontend/pages/item/[id]/edit.vue index ede78b8..a43abc6 100644 --- a/frontend/pages/item/[id]/edit.vue +++ b/frontend/pages/item/[id]/edit.vue @@ -18,7 +18,7 @@ const itemId = computed(() => route.params.id as string); const locationStore = useLocationStore(); - const locations = computed(() => locationStore.locations); + const locations = computed(() => locationStore.allLocations); const labelStore = useLabelStore(); const labels = computed(() => labelStore.labels); diff --git a/frontend/pages/items.vue b/frontend/pages/items.vue index a1f21a2..317a0f0 100644 --- a/frontend/pages/items.vue +++ b/frontend/pages/items.vue @@ -82,7 +82,7 @@ }); const locationsStore = useLocationStore(); - const locations = computed(() => locationsStore.locations); + const locations = computed(() => locationsStore.allLocations); const labelStore = useLabelStore(); const labels = computed(() => labelStore.labels); diff --git a/frontend/pages/location/[id].vue b/frontend/pages/location/[id].vue index d915e0b..d58a935 100644 --- a/frontend/pages/location/[id].vue +++ b/frontend/pages/location/[id].vue @@ -117,7 +117,7 @@ } const locationStore = useLocationStore(); - const locations = computed(() => locationStore.locations); + const locations = computed(() => locationStore.allLocations); // eslint-disable-next-line @typescript-eslint/no-explicit-any const parent = ref({}); diff --git a/frontend/stores/locations.ts b/frontend/stores/locations.ts index e2b2680..68295f6 100644 --- a/frontend/stores/locations.ts +++ b/frontend/stores/locations.ts @@ -3,7 +3,8 @@ import { LocationOutCount } from "~~/lib/api/types/data-contracts"; export const useLocationStore = defineStore("locations", { state: () => ({ - allLocations: null as LocationOutCount[] | null, + parents: null as LocationOutCount[] | null, + Locations: null as LocationOutCount[] | null, client: useUserApi(), }), getters: { @@ -12,21 +13,36 @@ export const useLocationStore = defineStore("locations", { * synched with the server by intercepting the API calls and updating on the * response */ - locations(state): LocationOutCount[] { - if (state.allLocations === null) { - Promise.resolve(this.refresh()); + parentLocations(state): LocationOutCount[] { + if (state.parents === null) { + Promise.resolve(this.refreshParents()); } - return state.allLocations; + return state.parents; + }, + allLocations(state): LocationOutCount[] { + if (state.Locations === null) { + Promise.resolve(this.refreshChildren()); + } + return state.Locations; }, }, actions: { - async refresh(): Promise { - const result = await this.client.locations.getAll(); + async refreshParents(): Promise { + const result = await this.client.locations.getAll({ filterChildren: true }); if (result.error) { return result; } - this.allLocations = result.data.items; + this.parents = result.data.items; + return result; + }, + async refreshChildren(): Promise { + const result = await this.client.locations.getAll({ filterChildren: false }); + if (result.error) { + return result; + } + + this.Locations = result.data.items; return result; }, },