Skip to content
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.

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

[24.0] Fix Multi-History status bar reactivity #17812

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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());
Copy link
Member

Choose a reason for hiding this comment

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

Debug leftover?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

absolutely... 😅 Thanks!

🦅 👁️ 😄


// 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
Loading