Skip to content

Commit

Permalink
Merge pull request #17812 from davelopez/24.0_fix_multi_history_counts
Browse files Browse the repository at this point in the history
[24.0] Fix Multi-History status bar reactivity
  • Loading branch information
mvdbeek authored Mar 28, 2024
2 parents 3669cae + 9b91fa5 commit 75a809e
Show file tree
Hide file tree
Showing 15 changed files with 238 additions and 102 deletions.
60 changes: 57 additions & 3 deletions client/src/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,72 @@ import { components } from "@/api/schema";
*/
export type HistorySummary = components["schemas"]["HistorySummary"];

export interface HistorySummaryExtended extends HistorySummary {
/**
* Contains minimal information about a History with additional content stats.
* This is a subset of information that can be relatively frequently updated after
* certain actions are performed on the history.
*/
export interface HistoryContentsStats {
id: string;
update_time: string;
size: number;
contents_active: components["schemas"]["HistoryActiveContentCounts"];
}

/**
* Contains summary information plus additional details about the contents and owner of a History.
* This is used by the client API to simplify the handling of History objects.
*
* Data returned by the API when requesting `?view=summary&keys=size,contents_active,user_id`.
*/
export interface HistorySummaryExtended extends HistorySummary, HistoryContentsStats {
user_id: string;
}

type HistoryDetailedModel = components["schemas"]["HistoryDetailed"];

/**
* Contains additional details about a History.
*
* Data returned by the API when requesting `?view=detailed`.
*/
export interface HistoryDetailed extends HistoryDetailedModel {
// TODO: these fields are not present in the backend schema model `HistoryDetailedModel` but are serialized by the API
// when requesting ?view=detailed. We should consider adding them to the backend schema.
email_hash?: string;
empty: boolean;
hid_counter: number;
}

type HistoryDetailedCommon = Omit<
HistoryDetailed,
"username" | "state" | "state_ids" | "state_details" | "email_hash" | "empty"
>;

/**
* Alternative representation of history details used by the client API.
* Shares most of the fields with HistoryDetailed but not all and adds some additional fields.
*
* Data returned by the API when requesting `?view=dev-detailed`.
*/
export type HistoryDetailed = components["schemas"]["HistoryDetailed"];
export interface HistoryDevDetailed extends HistoryDetailedCommon {
contents_active: components["schemas"]["HistoryActiveContentCounts"];
}

export type AnyHistory = HistorySummary | HistorySummaryExtended | HistoryDetailed;
/**
* Contains all available information about a History.
*/
export type HistoryExtended = HistoryDevDetailed & HistoryDetailed;

/**
* Represents any amount of information about a History with the minimal being a HistorySummary.
*/
export type AnyHistory =
| HistorySummary
| HistorySummaryExtended
| HistoryDetailed
| HistoryDevDetailed
| HistoryExtended;

/**
* Contains minimal information about a HistoryContentItem.
Expand Down
11 changes: 6 additions & 5 deletions client/src/components/History/CurrentHistory/HistoryCounter.vue
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,13 @@ import prettyBytes from "pretty-bytes";
import { computed, onMounted, ref, toRef } from "vue";
import { useRouter } from "vue-router/composables";
import type { HistorySummary } from "@/api";
import type { HistorySummaryExtended } from "@/api";
import { HistoryFilters } from "@/components/History/HistoryFilters.js";
import { useConfig } from "@/composables/config";
import { useHistoryContentStats } from "@/composables/historyContentStats";
import { useStorageLocationConfiguration } from "@/composables/storageLocation";
import { useUserStore } from "@/stores/userStore";
import { useDetailedHistory } from "./usesDetailedHistory";
import PreferredStorePopover from "./PreferredStorePopover.vue";
import SelectPreferredStore from "./SelectPreferredStore.vue";
Expand All @@ -26,7 +25,7 @@ library.add(faDatabase, faEyeSlash, faHdd, faMapMarker, faSync, faTrash);
const props = withDefaults(
defineProps<{
history: HistorySummary;
history: HistorySummaryExtended;
isWatching?: boolean;
lastChecked: Date;
filterText?: string;
Expand All @@ -47,7 +46,9 @@ const emit = defineEmits(["update:filter-text", "reloadContents"]);
const router = useRouter();
const { config } = useConfig();
const { currentUser } = storeToRefs(useUserStore());
const { historySize, numItemsActive, numItemsDeleted, numItemsHidden } = useDetailedHistory(toRef(props, "history"));
const { historySize, numItemsActive, numItemsDeleted, numItemsHidden } = useHistoryContentStats(
toRef(props, "history")
);
const reloadButtonLoading = ref(false);
const reloadButtonTitle = ref("");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,26 @@ import { FontAwesomeIcon } from "@fortawesome/vue-fontawesome";
import { BDropdown, BDropdownItem, BDropdownText, BModal } from "bootstrap-vue";
import { toRef } from "vue";
import type { HistorySummary } from "@/api";
import { useDetailedHistory } from "@/components/History/CurrentHistory/usesDetailedHistory";
import type { HistorySummary, HistorySummaryExtended } from "@/api";
import {
deleteAllHiddenContent,
purgeAllDeletedContent,
unhideAllHiddenContent,
} from "@/components/History/model/crud";
import { iframeRedirect } from "@/components/plugins/legacyNavigation";
import { useHistoryContentStats } from "@/composables/historyContentStats";
library.add(faCog);
interface Props {
history: HistorySummary;
history: HistorySummaryExtended;
}
const props = defineProps<Props>();
const emit = defineEmits(["update:operation-running"]);
const { numItemsDeleted, numItemsHidden } = useDetailedHistory(toRef(props, "history"));
const { numItemsDeleted, numItemsHidden } = useHistoryContentStats(toRef(props, "history"));
function onCopy() {
iframeRedirect("/dataset/copy_datasets");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,18 @@ import { library } from "@fortawesome/fontawesome-svg-core";
import { faCheckSquare, faCompress } from "@fortawesome/free-solid-svg-icons";
import { FontAwesomeIcon } from "@fortawesome/vue-fontawesome";
import type { HistorySummary } from "@/api";
import type { HistorySummaryExtended } from "@/api";
import DefaultOperations from "@/components/History/CurrentHistory/HistoryOperations/DefaultOperations.vue";
library.add(faCheckSquare, faCompress);
interface Props {
history: HistorySummary;
history: HistorySummaryExtended;
hasMatches: boolean;
expandedCount: number;
showSelection: boolean;
isMultiViewItem: boolean;
}
const props = defineProps<Props>();
Expand Down Expand Up @@ -60,6 +61,7 @@ function onUpdateOperationStatus(updateTime: number) {
</BButtonGroup>

<DefaultOperations
v-if="!isMultiViewItem"
v-show="!showSelection"
:history="history"
@update:operation-running="onUpdateOperationStatus" />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<template>
<section v-if="hasSelection">
<section v-if="hasSelection && !isMultiViewItem">
<b-dropdown text="Selection" size="sm" variant="primary" data-description="selected content menu" no-flip>
<template v-slot:button-content>
<span v-if="selectionMatchesQuery" data-test-id="all-filter-selected">
Expand Down Expand Up @@ -178,6 +178,7 @@ export default {
contentSelection: { type: Map, required: true },
selectionSize: { type: Number, required: true },
isQuerySelection: { type: Boolean, required: true },
isMultiViewItem: { type: Boolean, required: true },
totalItemsInQuery: { type: Number, default: 0 },
},
setup() {
Expand Down
15 changes: 12 additions & 3 deletions client/src/components/History/CurrentHistory/HistoryPanel.vue
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { BAlert } from "bootstrap-vue";
import { storeToRefs } from "pinia";
import { computed, onMounted, type Ref, ref, set as VueSet, unref, watch } from "vue";
import type { HistorySummary } from "@/api";
import type { HistorySummaryExtended } from "@/api";
import { copyDataset } from "@/api/datasets";
import ExpandedItems from "@/components/History/Content/ExpandedItems";
import SelectedItems from "@/components/History/Content/SelectedItems";
Expand Down Expand Up @@ -47,7 +47,7 @@ interface BackendFilterError {
interface Props {
listOffset?: number;
history: HistorySummary;
history: HistorySummaryExtended;
filter?: string;
canEditHistory?: boolean;
filterable?: boolean;
Expand Down Expand Up @@ -294,6 +294,7 @@ async function onDelete(item: HistoryItem, recursive = false) {
try {
await deleteContent(item, { recursive: recursive });
updateContentStats();
} finally {
isLoading.value = false;
}
Expand All @@ -315,6 +316,7 @@ async function onUndelete(item: HistoryItem) {
try {
await updateContentFields(item, { deleted: false });
updateContentStats();
} finally {
isLoading.value = false;
}
Expand All @@ -326,11 +328,16 @@ async function onUnhide(item: HistoryItem) {
try {
await updateContentFields(item, { visible: true });
updateContentStats();
} finally {
isLoading.value = false;
}
}
function updateContentStats() {
historyStore.updateContentStats(props.history.id);
}
function reloadContents() {
startWatchingHistory();
}
Expand Down Expand Up @@ -542,6 +549,7 @@ function setItemDragstart(
<HistoryOperations
v-if="canEditHistory"
:history="history"
:is-multi-view-item="isMultiViewItem"
:show-selection="showSelection"
:expanded-count="expandedCount"
:has-matches="hasMatches(historyItems)"
Expand All @@ -551,6 +559,7 @@ function setItemDragstart(
<template v-slot:selection-operations>
<HistorySelectionOperations
:history="history"
:is-multi-view-item="isMultiViewItem"
:filter-text="filterText"
:content-selection="selectedItems"
:selection-size="selectionSize"
Expand All @@ -570,7 +579,7 @@ function setItemDragstart(
</template>
</HistoryOperations>

<SelectionChangeWarning :query-selection-break="querySelectionBreak" />
<SelectionChangeWarning v-if="!isMultiViewItem" :query-selection-break="querySelectionBreak" />

<OperationErrorDialog
v-if="operationError"
Expand Down

This file was deleted.

10 changes: 4 additions & 6 deletions client/src/components/History/HistoryScrollList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { storeToRefs } from "pinia";
import { computed, onMounted, onUnmounted, ref, watch } from "vue";
import { useRouter } from "vue-router/composables";
import type { HistoryDetailed, HistorySummary } from "@/api";
import { type AnyHistory, type HistorySummary, userOwnsHistory } from "@/api";
import { useAnimationFrameResizeObserver } from "@/composables/sensors/animationFrameResizeObserver";
import { useAnimationFrameScroll } from "@/composables/sensors/animationFrameScroll";
import { useHistoryStore } from "@/stores/historyStore";
Expand Down Expand Up @@ -121,11 +121,9 @@ watch(
/** `historyStore` histories for current user */
const historiesProxy = ref<HistorySummary[]>([]);
watch(
() => histories.value as HistoryDetailed[],
(h: HistoryDetailed[]) => {
historiesProxy.value = h.filter(
(h) => !h.user_id || (!currentUser.value?.isAnonymous && h.user_id === currentUser.value?.id)
);
() => histories.value as AnyHistory[],
(h: AnyHistory[]) => {
historiesProxy.value = h.filter((h) => userOwnsHistory(currentUser.value, h));
},
{
immediate: true,
Expand Down
38 changes: 22 additions & 16 deletions client/src/components/History/Multiple/MultipleView.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { mount } from "@vue/test-utils";
import axios from "axios";
import MockAdapter from "axios-mock-adapter";
import MockUserHistories from "components/providers/MockUserHistories";
import flushPromises from "flush-promises";
import { createPinia } from "pinia";
import { useHistoryStore } from "stores/historyStore";
Expand All @@ -24,13 +23,24 @@ const getFakeHistorySummaries = (num, selectedIndex) => {
const currentUser = { id: USER_ID };

describe("MultipleView", () => {
async function setUpWrapper(UserHistoriesMock, count, currentHistoryId) {
const axiosMock = new MockAdapter(axios);
axiosMock.onGet(`api/histories/${FIRST_HISTORY_ID}`).reply(200, {});
let axiosMock;

beforeEach(() => {
axiosMock = new MockAdapter(axios);
});

afterEach(() => {
axiosMock.reset();
});

async function setUpWrapper(count, currentHistoryId) {
const fakeSummaries = getFakeHistorySummaries(count, 0);
for (const summary of fakeSummaries) {
axiosMock.onGet(`api/histories/${summary.id}`).reply(200, summary);
}
const wrapper = mount(MultipleView, {
pinia: createPinia(),
stubs: {
UserHistories: UserHistoriesMock,
HistoryPanel: true,
icon: { template: "<div></div>" },
},
Expand All @@ -41,21 +51,22 @@ describe("MultipleView", () => {
userStore.currentUser = currentUser;

const historyStore = useHistoryStore();
historyStore.setHistories(getFakeHistorySummaries(count, 0));
historyStore.setHistories(fakeSummaries);
historyStore.setCurrentHistoryId(currentHistoryId);

await flushPromises();

return { wrapper, axiosMock };
return wrapper;
}

it("more than 4 histories should not show the current history", async () => {
const count = 8;
const currentHistoryId = FIRST_HISTORY_ID;

// Set up UserHistories and wrapper
const UserHistoriesMock = MockUserHistories({ id: currentHistoryId }, getFakeHistorySummaries(count, 0), false);
const { wrapper, axiosMock } = await setUpWrapper(UserHistoriesMock, count, currentHistoryId);
const wrapper = await setUpWrapper(count, currentHistoryId);

console.log(wrapper.html());

// Test: current (first) history should not be shown because only 4 latest are shown by default
expect(wrapper.find("button[title='Current History']").exists()).toBeFalsy();
Expand All @@ -65,21 +76,16 @@ describe("MultipleView", () => {
expect(wrapper.find("div[title='Currently showing 4 most recently updated histories']").exists()).toBeTruthy();

expect(wrapper.find("[data-description='open select histories modal']").exists()).toBeTruthy();

axiosMock.reset();
});

it("less than or equal to 4 histories should not show the current history", async () => {
it("less than 4 histories should show the current history", async () => {
const count = 3;
const currentHistoryId = FIRST_HISTORY_ID;

// Set up UserHistories and wrapper
const UserHistoriesMock = MockUserHistories({ id: currentHistoryId }, getFakeHistorySummaries(count, 0), false);
const { wrapper, axiosMock } = await setUpWrapper(UserHistoriesMock, count, currentHistoryId);
const wrapper = await setUpWrapper(count, currentHistoryId);

// Test: current (first) history should be shown because only 4 latest are shown by default, and count = 3
expect(wrapper.find("button[title='Current History']").exists()).toBeTruthy();

axiosMock.reset();
});
});
Loading

0 comments on commit 75a809e

Please sign in to comment.