Merge pull request #826 from nextcloud/fix/delete-view-rows

fix: properly delete rows in views

cypress/e2e/view.cy.js

@@ -1,4 +1,5 @@
 let localUser
+const title = 'Test view'
 
 describe('Interact with views', () => {
 
@@ -11,11 +12,8 @@ describe('Interact with views', () => {
 	beforeEach(function() {
 		cy.login(localUser)
 		cy.visit('apps/tables')
-	})
 
-	it('Create view and insert rows in the view', () => {
-		const title = 'View for adding rows'
-		cy.createTable('View filtering test table')
+		cy.createTable('View test table')
 		cy.createTextLineColumn('title', null, null, true)
 		cy.createSelectionColumn('selection', ['sel1', 'sel2', 'sel3', 'sel4'], null, false)
 
@@ -37,41 +35,82 @@ describe('Interact with views', () => {
 		cy.fillInValueSelection('selection', 'sel2')
 		cy.get('button').contains('Save').click()
 
+		// create view
 		cy.get('[data-cy="customTableAction"] button').click()
-		cy.get('.v-popper__popper li button span').contains('Create view').click({ force: true })
-		cy.get('.modal-container #settings-section_title input').type(title)
+		cy.get('[data-cy="dataTableCreateViewBtn"]').contains('Create view').click({ force: true })
+		cy.get('[data-cy="viewSettingsDialogSection"] input').type(title)
+	})
 
+	it('Create view and insert rows in the view', () => {
 		// ## add filter
-		cy.get('button').contains('Add new filter group').click()
-		cy.get('.modal-container .filter-group .v-select.select').eq(0).click()
+		cy.get('[data-cy="filterFormFilterGroupBtn"]').contains('Add new filter group').click()
+		cy.get('[data-cy="filterEntryColumn"]').click()
 		cy.get('ul.vs__dropdown-menu li span[title="selection"]').click()
-		cy.get('.modal-container .filter-group .v-select.select').eq(1).click()
+		cy.get('[data-cy="filterEntryOperator"]').click()
 		cy.get('ul.vs__dropdown-menu li span[title="Is equal"]').click()
-		cy.get('.modal-container .filter-group .v-select.select').eq(2).click()
+		cy.get('[data-cy="filterEntrySeachValue"]').click()
 		cy.get('ul.vs__dropdown-menu li span[title="sel2"]').click()
 
 		// ## save view
 		cy.intercept({ method: 'POST', url: '**/apps/tables/view' }).as('createView')
 		cy.intercept({ method: 'PUT', url: '**/apps/tables/view/*' }).as('updateView')
-		cy.contains('button', 'Create View').click()
+		cy.get('[data-cy="modifyViewBtn"]').contains('Create View').click()
 		cy.wait('@createView')
 		cy.wait('@updateView')
-		cy.contains('.app-navigation-entry-link span', title).should('exist')
+		cy.get('[data-cy="navigationViewItem"]').contains(title).should('exist')
 
 		const expected = ['sevenths row', 'second row']
 		expected.forEach(item => {
-			cy.get('.custom-table table tr td div').contains(item).should('be.visible')
+			cy.get('[data-cy="customTableRow"] td div').contains(item).should('be.visible')
 		})
 
-		// Add new row in the views
-		cy.get('button').contains('Create row').click()
+		// Add new row in the view
+		cy.get('[data-cy="createRowBtn"]').contains('Create row').click()
 		cy.fillInValueTextLine('title', 'new row')
 		cy.fillInValueSelection('selection', 'sel2')
-		cy.get('button').contains('Save').click()
+		cy.get('[data-cy="createRowSaveButton"]').contains('Save').click()
 
 		expected.push('new row')
 		expected.forEach(item => {
-			cy.get('.custom-table table tr td div').contains(item).should('be.visible')
+			cy.get('[data-cy="customTableRow"] td div').contains(item).should('be.visible')
 		})
 	})
+
+	it('Create view and update rows in the view', () => {
+		// ## save view
+		cy.intercept({ method: 'POST', url: '**/apps/tables/view' }).as('createView')
+		cy.intercept({ method: 'PUT', url: '**/apps/tables/view/*' }).as('updateView')
+		cy.get('[data-cy="modifyViewBtn"]').contains('Create View').click()
+		cy.wait('@createView')
+		cy.wait('@updateView')
+		cy.get('[data-cy="navigationViewItem"]').contains(title).should('exist')
+
+		// Update rows in the view
+		cy.get('[data-cy="customTableRow"]').contains('first row').parent().parent().find('[data-cy="editRowBtn"]').click()
+		cy.get('[data-cy="editRowModal"] input').first().clear().type('Changed row')
+		cy.get('[data-cy="editRowSaveButton"]').contains('Save').click()
+
+		cy.get('[data-cy="editRowModal"]').should('not.exist')
+		cy.get('[data-cy="customTableRow"]').contains('first row').should('not.exist')
+		cy.get('[data-cy="customTableRow"]').contains('Changed row').should('exist')
+	})
+
+	it('Create view and delete rows in the view', () => {
+
+		// ## save view
+		cy.intercept({ method: 'POST', url: '**/apps/tables/view' }).as('createView')
+		cy.intercept({ method: 'PUT', url: '**/apps/tables/view/*' }).as('updateView')
+		cy.get('[data-cy="modifyViewBtn"]').contains('Create View').click()
+		cy.wait('@createView')
+		cy.wait('@updateView')
+		cy.get('[data-cy="navigationViewItem"]').contains(title).should('exist')
+
+		// Delete rows in the view
+		cy.get('[data-cy="customTableRow"]').contains('first row').parent().parent().find('[data-cy="editRowBtn"]').click()
+		cy.get('[data-cy="editRowModal"] [data-cy="editRowDeleteButton"]').contains('Delete').click()
+		cy.get('[data-cy="editRowModal"] [data-cy="editRowEditConfirmButton"]').contains('I really want to delete this row!').click()
+
+		cy.get('[data-cy="editRowModal"]').should('not.exist')
+		cy.get('[data-cy="customTableRow"]').contains('first row').should('not.exist')
+	})
 })

lib/Service/RowService.php

@@ -4,7 +4,6 @@
 
 use OCA\Tables\Db\Column;
 use OCA\Tables\Db\ColumnMapper;
-use OCA\Tables\Db\LegacyRowMapper;
 use OCA\Tables\Db\Row2;
 use OCA\Tables\Db\Row2Mapper;
 use OCA\Tables\Db\Table;
@@ -28,17 +27,15 @@
  * @psalm-import-type TablesRow from ResponseDefinitions
  */
 class RowService extends SuperService {
-	private LegacyRowMapper $mapper;
 	private ColumnMapper $columnMapper;
 	private ViewMapper $viewMapper;
 	private TableMapper $tableMapper;
 	private Row2Mapper $row2Mapper;
 	private array $tmpRows = []; // holds already loaded rows as a small cache
 
 	public function __construct(PermissionsService $permissionsService, LoggerInterface $logger, ?string $userId,
-		LegacyRowMapper    $mapper, ColumnMapper $columnMapper, ViewMapper $viewMapper, TableMapper $tableMapper, Row2Mapper $row2Mapper) {
+		ColumnMapper $columnMapper, ViewMapper $viewMapper, TableMapper $tableMapper, Row2Mapper $row2Mapper) {
 		parent::__construct($logger, $userId, $permissionsService);
-		$this->mapper = $mapper;
 		$this->columnMapper = $columnMapper;
 		$this->viewMapper = $viewMapper;
 		$this->tableMapper = $tableMapper;
@@ -438,8 +435,7 @@ public function delete(int $id, ?int $viewId, string $userId): Row2 {
 				$this->logger->error($e->getMessage(), ['exception' => $e]);
 				throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage());
 			}
-			$rowIds = $this->mapper->getRowIdsOfView($view, $userId);
-			if(!in_array($id, $rowIds)) {
+			if(!$this->row2Mapper->isRowInViewPresent($id, $view, $userId)) {
 				$e = new \Exception('Update row is not allowed.');
 				$this->logger->error($e->getMessage(), ['exception' => $e]);
 				throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage());

src/modules/main/partials/editViewPartials/filter/FilterEntry.vue

@@ -6,15 +6,17 @@
 				class="select-field"
 				:options="columns"
 				label="title"
-				:placeholder="t('tables', 'Column')" />
+				:placeholder="t('tables', 'Column')"
+				data-cy="filterEntryColumn" />
 		</div>
 		<div class="fix-col-2">
 			<NcSelect
 				v-if="selectedColumn"
 				v-model="selectedOperator"
 				class="select-field"
 				:options="operators"
-				:placeholder="t('tables', 'Operator')" />
+				:placeholder="t('tables', 'Operator')"
+				data-cy="filterEntryOperator" />
 		</div>
 		<div class="fix-col-2">
 			<NcSelect
@@ -23,6 +25,7 @@
 				class="select-field"
 				:options="magicFields"
 				:placeholder="getValuePlaceholder"
+				data-cy="filterEntrySeachValue"
 				@search="v => term = v" />
 		</div>
 		<div class="fix-col-2 actions">

src/modules/main/partials/editViewPartials/filter/FilterForm.vue

@@ -21,6 +21,7 @@
 			:close-after-click="true"
 			:aria-label="t('tables', 'Add new filter group')"
 			type="tertiary"
+			data-cy="filterFormFilterGroupBtn"
 			@click="addFilterGroup">
 			{{ t('tables', 'Add new filter group') }}
 			<template #icon>

src/modules/main/partials/editViewPartials/filter/FilterGroup.vue

@@ -1,5 +1,5 @@
 <template>
-	<div class="filter-group">
+	<div class="filter-group" data-cy="filterGroupSection">
 		<div class="group-text">
 			{{ t('tables', '... that meet all of the following conditions') }}
 		</div>

src/modules/main/sections/DataTable.vue

@@ -94,13 +94,13 @@
 						</NcActionButton>
 						<NcActionButton v-if="canManageElement(table) "
 							:close-after-click="true"
-							@click="$emit('create-view')">
+							data-cy="dataTableCreateViewBtn" @click="$emit('create-view')">
 							<template #icon>
 								<PlaylistPlus :size="20" decorative />
 							</template>
 							{{ t('tables', 'Create view') + (isViewSettingSet ? '*' : '') }}
 						</NcActionButton>
-						<NcActionButton v-if="canManageTable(table)" :close-after-click="true" @click="$emit('create-column')">
+						<NcActionButton v-if="canManageTable(table)" :close-after-click="true" data-cy="dataTableCreateColumnBtn" @click="$emit('create-column')">
 							<template #icon>
 								<TableColumnPlusAfter :size="20" decorative title="" />
 							</template>
@@ -110,15 +110,15 @@
 						<NcActionCaption :title="t('tables', 'Integration')" />
 						<NcActionButton v-if="canCreateRowInElement(table)"
 							:close-after-click="true"
-							@click="$emit('import', table)">
+							data-cy="dataTableExportBtn" @click="$emit('import', table)">
 							<template #icon>
 								<Import :size="20" decorative title="Import" />
 							</template>
 							{{ t('tables', 'Import') }}
 						</NcActionButton>
 						<NcActionButton v-if="canReadData(table)" :close-after-click="true"
 							icon="icon-download"
-							@click="$emit('download-csv')">
+							data-cy="dataTableExportBtn" @click="$emit('download-csv')">
 							{{ t('tables', 'Export as CSV') }}
 						</NcActionButton>
 						<NcActionButton v-if="canShareElement(table)"

src/modules/modals/EditRow.vue

@@ -20,10 +20,11 @@
 			<div class="row">
 				<div class="fix-col-4 space-T" :class="{'justify-between': showDeleteButton, 'end': !showDeleteButton}">
 					<div v-if="showDeleteButton">
-						<NcButton v-if="!prepareDeleteRow" :aria-label="t('tables', 'Delete')" type="error" @click="prepareDeleteRow = true">
+						<NcButton v-if="!prepareDeleteRow" :aria-label="t('tables', 'Delete')" type="error" data-cy="editRowDeleteButton" @click="prepareDeleteRow = true">
 							{{ t('tables', 'Delete') }}
 						</NcButton>
 						<NcButton v-if="prepareDeleteRow"
+							data-cy="editRowEditConfirmButton"
 							:wide="true"
 							:aria-label="t('tables', 'I really want to delete this row!')"
 							type="error"

src/modules/modals/ViewSettings.vue

@@ -1,10 +1,10 @@
 <template>
-	<NcAppSettingsDialog :open.sync="open" :show-navigation="true" :title="createView ? t('tables', 'Create view') : t('tables', 'Edit view')">
+	<NcAppSettingsDialog :open.sync="open" :show-navigation="true" data-cy="viewSettingsDialog" :title="createView ? t('tables', 'Create view') : t('tables', 'Edit view')">
 		<NcAppSettingsSection v-if="columns === null" id="loading" title="">
 			<div class="icon-loading" />
 		</NcAppSettingsSection>
 		<!--title & emoji-->
-		<NcAppSettingsSection v-if="columns != null" id="title" :title="t('tables', 'Title')">
+		<NcAppSettingsSection v-if="columns != null" id="title" :title="t('tables', 'Title')" data-cy="viewSettingsDialogSection">
 			<div class="col-4" style="display: inline-flex;">
 				<NcEmojiPicker :close-on-select="true" @select="setIcon">
 					<NcButton type="tertiary"
@@ -53,7 +53,7 @@
 						{{ createNewViewText }}
 					</NcButton>
 				</div>
-				<NcButton v-if="!localLoading" type="primary" :aria-label="saveText" @click="saveView()">
+				<NcButton v-if="!localLoading" type="primary" :aria-label="saveText" data-cy="modifyViewBtn" @click="saveView()">
 					{{ saveText }}
 				</NcButton>
 			</div>

src/modules/navigation/partials/NavigationViewItem.vue

@@ -1,5 +1,6 @@
 <template>
 	<NcAppNavigationItem v-if="view"
+		data-cy="navigationViewItem"
 		:name="view.title"
 		:class="{active: activeView && view.id === activeView.id}"
 		:force-menu="true"

src/shared/components/ncTable/partials/TableRow.vue

@@ -10,7 +10,7 @@
 				:value="getCellValue(col)" />
 		</td>
 		<td v-if="config.showActions" :class="{sticky: config.showActions}">
-			<NcButton v-if="config.canEditRows || config.canDeleteRows" type="primary" :aria-label="t('tables', 'Edit row')" @click="$emit('edit-row', row.id)">
+			<NcButton v-if="config.canEditRows || config.canDeleteRows" type="primary" :aria-label="t('tables', 'Edit row')" data-cy="editRowBtn" @click="$emit('edit-row', row.id)">
 				<template #icon>
 					<Pencil :size="20" />
 				</template>

src/shared/components/ncTable/sections/CustomTable.vue

@@ -21,6 +21,7 @@
 			<tbody>
 				<TableRow v-for="(row, index) in getSearchedAndFilteredAndSortedRows"
 					:key="index"
+					data-cy="customTableRow"
 					:row="row"
 					:columns="columns"
 					:selected="isRowSelected(row.id)"

src/shared/components/ncTable/sections/Options.vue

@@ -7,6 +7,7 @@
 					:aria-label="t('tables', 'Create row')"
 					:close-after-click="true"
 					type="tertiary"
+					data-cy="createRowBtn"
 					@click="$emit('create-row')">
 					{{ t('tables', 'Create row') }}
 					<template #icon>
@@ -17,6 +18,7 @@
 					:close-after-click="true"
 					:aria-label="t('tables', 'Create Row')"
 					type="tertiary"
+					data-cy="createRowBtn"
 					@click="$emit('create-row')">
 					<template #icon>
 						<Plus :size="25" />