Merge pull request #533 from nextcloud/fix/529

nextcloud · Sep 12, 2023 · 3f63c14 · 3f63c14
2 parents fdccba3 + f88b60a
commit 3f63c14
Show file tree

Hide file tree

Showing 12 changed files with 288 additions and 36 deletions.
diff --git a/.github/workflows/cypress.yml b/.github/workflows/cypress.yml
@@ -92,6 +92,8 @@ jobs:
         run: |
           npm ci
           TESTING=true npm run build --if-present
+          composer i --no-dev
+
 
       - name: Use Node.js ${{ matrix.node-version }}
         uses: actions/setup-node@v3
@@ -126,6 +128,9 @@ jobs:
           export OC_PASS=1234561
           php occ user:add --password-from-env user1
           php occ user:add --password-from-env user2
+          ./occ group:add shareTestGroup
+          ./occ group:adduser shareTestGroup user1
+          ./occ group:adduser shareTestGroup user2
           curl -v http://127.0.0.1:8081/index.php/login
           cat data/nextcloud.log
 

diff --git a/appinfo/info.xml b/appinfo/info.xml
@@ -43,7 +43,7 @@ Have a good time and manage whatever you want.
 		<database>pgsql</database>
 		<database>mysql</database>
 		<database>sqlite</database>
-        <nextcloud min-version="25" max-version="27"/>
+        <nextcloud min-version="25" max-version="28"/>
     </dependencies>
     <commands>
 		<command>OCA\Tables\Command\ListTables</command>

diff --git a/cypress/e2e/tables-unified-search.cy.js b/cypress/e2e/tables-unified-search.cy.js
@@ -0,0 +1,124 @@
+let localUser
+let localUser2
+
+describe('The Home Page', () => {
+
+	before(function() {
+		cy.createRandomUser().then(user => {
+			localUser = user
+		})
+		cy.createRandomUser().then(user => {
+			localUser2 = user
+		})
+	})
+
+	beforeEach(function() {
+		cy.login(localUser)
+		cy.visit('apps/tables')
+	})
+
+	it('Create a table and view and search via unified search for it', () => {
+		cy.loadTable('Tutorial')
+		cy.createView('asdfghjkl')
+		cy.unifiedSearch('HJK')
+		cy.loadTable('Tutorial')
+		cy.unifiedSearch('asd')
+		cy.loadTable('Tutorial')
+		cy.unifiedSearch('Tutorial')
+	})
+
+	it('Search for shared table via user share', () => {
+		cy.login(localUser)
+		cy.visit('apps/tables')
+
+		// create table to share
+		cy.contains('.app-menu-entry--label', 'Tables').click()
+		cy.createTable('Share for user')
+		cy.createTextLineColumn('any', true)
+
+		cy.clickOnTableThreeDotMenu('Share')
+
+		cy.intercept({ method: 'GET', url: '**/ocs/v2.php/apps/files_sharing/api/v1/sharees*' }).as('searchShareUsers')
+		cy.get('.sharing input').type(localUser2.userId)
+		cy.wait('@searchShareUsers')
+		cy.get('.sharing input').type('{enter}')
+
+		cy.get('h3').contains('Shares').parent().find('ul').contains(localUser2.userId).should('exist')
+
+		cy.login(localUser2)
+		cy.visit('apps/tables')
+		cy.unifiedSearch('Share for user')
+	})
+
+	it('Search for shared view via user share', () => {
+		cy.login(localUser)
+		cy.visit('apps/tables')
+
+		// create table to share
+		cy.contains('.app-menu-entry--label', 'Tables').click()
+		cy.createTable('Share for user')
+		cy.createTextLineColumn('any', true)
+		cy.createView('ShareView1')
+
+		cy.clickOnTableThreeDotMenu('Share')
+
+		cy.intercept({ method: 'GET', url: '**/ocs/v2.php/apps/files_sharing/api/v1/sharees*' }).as('searchShareUsers')
+		cy.get('.sharing input').type(localUser2.userId)
+		cy.wait('@searchShareUsers')
+		cy.get('.sharing input').type('{enter}')
+
+		cy.get('h3').contains('Shares').parent().find('ul').contains(localUser2.userId).should('exist')
+
+		cy.login(localUser2)
+		cy.visit('apps/tables')
+		cy.unifiedSearch('ShareView1')
+	})
+
+	it('Search for shared table via group share', () => {
+		cy.login({ userId: 'user1', password: '1234561' })
+		cy.visit('apps/tables')
+
+		// create table to share
+		cy.contains('.app-menu-entry--label', 'Tables').click()
+		cy.createTable('Share for group')
+		cy.createTextLineColumn('any', true)
+
+		cy.clickOnTableThreeDotMenu('Share')
+
+		cy.intercept({ method: 'GET', url: '**/ocs/v2.php/apps/files_sharing/api/v1/sharees*' }).as('searchShareUsers')
+		cy.get('.sharing input').type('shareTestGroup')
+		cy.wait('@searchShareUsers')
+		cy.get('.sharing input').type('{enter}')
+
+		cy.get('h3').contains('Shares').parent().find('ul').contains('shareTestGroup').should('exist')
+
+		cy.login({ userId: 'user2', password: '1234561' })
+		cy.visit('apps/tables')
+		cy.unifiedSearch('Share for group')
+	})
+
+	it('Search for shared view via group share', () => {
+		cy.login({ userId: 'user1', password: '1234561' })
+		cy.visit('apps/tables')
+
+		// create table to share
+		cy.contains('.app-menu-entry--label', 'Tables').click()
+		cy.createTable('Share for group')
+		cy.createTextLineColumn('any', true)
+		cy.createView('ShareView2')
+
+		cy.clickOnTableThreeDotMenu('Share')
+
+		cy.intercept({ method: 'GET', url: '**/ocs/v2.php/apps/files_sharing/api/v1/sharees*' }).as('searchShareUsers')
+		cy.get('.sharing input').type('shareTestGroup')
+		cy.wait('@searchShareUsers')
+		cy.get('.sharing input').type('{enter}')
+
+		cy.get('h3').contains('Shares').parent().find('ul').contains('shareTestGroup').should('exist')
+
+		cy.login({ userId: 'user2', password: '1234561' })
+		cy.visit('apps/tables')
+		cy.unifiedSearch('ShareView2')
+	})
+
+})
diff --git a/cypress/support/commands.js b/cypress/support/commands.js
@@ -36,13 +36,35 @@ Cypress.Commands.add('createTable', (title) => {
 	cy.get('.modal__content input[type="text"]').clear().type(title)
 	cy.contains('button', 'Create table').click()
 
-	cy.contains('h1', 'Test text-link').should('be.visible')
+	cy.contains('h1', title).should('be.visible')
+})
+
+Cypress.Commands.add('createView', (title) => {
+	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.contains('button', 'Create View').click()
+
+	cy.contains('.app-navigation-entry-link span', title).should('exist')
+})
+
+Cypress.Commands.add('clickOnTableThreeDotMenu', (optionName) => {
+	cy.get('[data-cy="customTableAction"] button').click()
+	cy.get('.v-popper__popper li button span').contains(optionName).click({ force: true })
 })
 
 Cypress.Commands.add('loadTable', (name) => {
 	cy.get('.app-navigation-entry-link').contains(name).click({ force: true })
 })
 
+Cypress.Commands.add('unifiedSearch', (term) => {
+	cy.get('#unified-search').click()
+	cy.get('#unified-search__input').type(term)
+	cy.get('.unified-search__results .unified-search__result-line-one span').contains(term, { matchCase: false }).should('exist')
+})
+
 Cypress.Commands.add('createTextLinkColumn', (title, ressourceProvider, firstColumn) => {
 	if (firstColumn) {
 		cy.get('.button-vue__text').contains('Create column').click({ force: true })
@@ -68,6 +90,21 @@ Cypress.Commands.add('createTextLinkColumn', (title, ressourceProvider, firstCol
 	cy.get('.custom-table table tr th .cell').contains(title).should('exist')
 })
 
+Cypress.Commands.add('createTextLineColumn', (title, firstColumn) => {
+	if (firstColumn) {
+		cy.get('.button-vue__text').contains('Create column').click({ force: true })
+	} else {
+		cy.get('[data-cy="customTableAction"] button').click()
+		cy.get('.v-popper__popper li button span').contains('Create column').click({ force: true })
+	}
+
+	cy.get('.modal-container').get('input[placeholder*="Enter a column title"]').clear().type(title)
+	cy.get('.modal-container button').contains('Save').click()
+
+	cy.wait(10).get('.toastify.toast-success').should('be.visible')
+	cy.get('.custom-table table tr th .cell').contains(title).should('exist')
+})
+
 Cypress.Commands.add('uploadFile', (fileName, mimeType, target) => {
 	return cy.fixture(fileName, 'binary')
 		.then(Cypress.Blob.binaryStringToBlob)

diff --git a/img/view-dark.svg b/img/view-dark.svg
diff --git a/img/view.svg b/img/view.svg
diff --git a/lib/Db/TableMapper.php b/lib/Db/TableMapper.php
@@ -2,6 +2,7 @@
 
 namespace OCA\Tables\Db;
 
+use OCA\Tables\Helper\UserHelper;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Db\MultipleObjectsReturnedException;
 use OCP\AppFramework\Db\QBMapper;
@@ -12,9 +13,11 @@
 /** @template-extends QBMapper<Table> */
 class TableMapper extends QBMapper {
 	protected string $table = 'tables_tables';
+	private UserHelper $userHelper;
 
-	public function __construct(IDBConnection $db) {
+	public function __construct(IDBConnection $db, UserHelper $userHelper) {
 		parent::__construct($db, $this->table, Table::class);
+		$this->userHelper = $userHelper;
 	}
 
 	/**
@@ -66,23 +69,37 @@ public function findAll(?string $userId = null): array {
 	 */
 	public function search(string $term = null, ?string $userId = null, ?int $limit = null, ?int $offset = null): array {
 		$qb = $this->db->getQueryBuilder();
-		$shareQuery = $this->db->getQueryBuilder();
+		$shareQueryTablesSharedViaUser = $this->db->getQueryBuilder();
+		$shareQueryTablesSharedViaGroup = $this->db->getQueryBuilder();
+		$userGroups = $this->userHelper->getGroupIdsForUser($userId);
 
 		// get table ids, that are shared with the given user
 		// only makes sense if a user is given, otherwise will always get all shares doubled
-		if ($userId !== null && $userId !== '') {
-			$shareQuery->selectDistinct('node_id')
+		if ($userId) {
+			$shareQueryTablesSharedViaUser->selectDistinct('node_id')
 				->from('tables_shares')
 				->andWhere($qb->expr()->eq('node_type', $qb->createNamedParameter('table', IQueryBuilder::PARAM_STR)))
+				->andWhere($qb->expr()->eq('receiver_type', $qb->createNamedParameter('user', IQueryBuilder::PARAM_STR)))
 				->andWhere($qb->expr()->eq('receiver', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)));
+
+			if($userGroups) {
+				$shareQueryTablesSharedViaGroup->selectDistinct('node_id')
+					->from('tables_shares')
+					->andWhere($qb->expr()->eq('node_type', $qb->createNamedParameter('table', IQueryBuilder::PARAM_STR)))
+					->andWhere($qb->expr()->eq('receiver_type', $qb->createNamedParameter('group', IQueryBuilder::PARAM_STR)))
+					->andWhere($qb->expr()->in('receiver', $qb->createNamedParameter($userGroups, IQueryBuilder::PARAM_STR_ARRAY)));
+			}
 		}
 
 		$qb->select('*')
 			->from($this->table);
 
-		if ($userId !== null && $userId !== '') {
+		if ($userId) {
 			$qb->andWhere($qb->expr()->eq('ownership', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)));
-			$qb->orWhere($shareQuery->expr()->in('id', $qb->createFunction($shareQuery->getSQL()), IQueryBuilder::PARAM_INT_ARRAY));
+			$qb->orWhere($shareQueryTablesSharedViaUser->expr()->in('id', $qb->createFunction($shareQueryTablesSharedViaUser->getSQL()), IQueryBuilder::PARAM_INT_ARRAY));
+			if($userGroups) {
+				$qb->orWhere($shareQueryTablesSharedViaGroup->expr()->in('id', $qb->createFunction($shareQueryTablesSharedViaGroup->getSQL()), IQueryBuilder::PARAM_INT_ARRAY));
+			}
 		}
 
 		if ($term) {
@@ -102,8 +119,6 @@ public function search(string $term = null, ?string $userId = null, ?int $limit
 			$qb->setFirstResult($offset);
 		}
 
-		$sql = $qb->getSQL();
-
 		return $this->findEntities($qb);
 	}
 }
diff --git a/lib/Db/ViewMapper.php b/lib/Db/ViewMapper.php
@@ -3,6 +3,7 @@
 namespace OCA\Tables\Db;
 
 use OCA\Tables\Errors\InternalError;
+use OCA\Tables\Helper\UserHelper;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Db\MultipleObjectsReturnedException;
 use OCP\AppFramework\Db\QBMapper;
@@ -13,12 +14,13 @@
 /** @template-extends QBMapper<View> */
 class ViewMapper extends QBMapper {
 	protected string $table = 'tables_views';
+	private UserHelper $userHelper;
+	private TableMapper $tableMapper;
 
-	protected TableMapper $tableMapper;
-
-	public function __construct(IDBConnection $db, TableMapper $tableMapper) {
+	public function __construct(IDBConnection $db, TableMapper $tableMapper, UserHelper $userHelper) {
 		parent::__construct($db, $this->table, View::class);
 		$this->tableMapper = $tableMapper;
+		$this->userHelper = $userHelper;
 	}
 
 
@@ -74,16 +76,28 @@ public function findAll(?int $tableId = null): array {
 	 */
 	public function search(string $term = null, ?string $userId = null, ?int $limit = null, ?int $offset = null): array {
 		$qb = $this->db->getQueryBuilder();
-		$shareViewQuery = $this->db->getQueryBuilder();
 		$shareTableQuery = $this->db->getQueryBuilder();
+		$shareQueryViewsSharedViaUser = $this->db->getQueryBuilder();
+		$shareQueryViewsSharedViaGroup = $this->db->getQueryBuilder();
+		$userGroups = $this->userHelper->getGroupIdsForUser($userId);
 
 		// get view ids, that are shared with the given user
 		// only makes sense if a user is given, otherwise will always get all shares doubled
-		if ($userId !== null && $userId !== '') {
-			$shareViewQuery->selectDistinct('node_id')
+		if ($userId) {
+			$shareQueryViewsSharedViaUser->selectDistinct('node_id')
 				->from('tables_shares')
 				->andWhere($qb->expr()->eq('node_type', $qb->createNamedParameter('view', IQueryBuilder::PARAM_STR)))
+				->andWhere($qb->expr()->eq('receiver_type', $qb->createNamedParameter('user', IQueryBuilder::PARAM_STR)))
 				->andWhere($qb->expr()->eq('receiver', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)));
+
+			if($userGroups) {
+				$shareQueryViewsSharedViaGroup->selectDistinct('node_id')
+					->from('tables_shares')
+					->andWhere($qb->expr()->eq('node_type', $qb->createNamedParameter('view', IQueryBuilder::PARAM_STR)))
+					->andWhere($qb->expr()->eq('receiver_type', $qb->createNamedParameter('group', IQueryBuilder::PARAM_STR)))
+					->andWhere($qb->expr()->in('receiver', $qb->createNamedParameter($userGroups, IQueryBuilder::PARAM_STR_ARRAY)));
+			}
+
 			$shareTableQuery->selectDistinct('node_id')
 				->from('tables_shares')
 				->andWhere($qb->expr()->eq('node_type', $qb->createNamedParameter('table', IQueryBuilder::PARAM_STR)))
@@ -94,10 +108,12 @@ public function search(string $term = null, ?string $userId = null, ?int $limit
 			->from($this->table, 'v')
 			->leftJoin('v', 'tables_tables', 't', 't.id = v.table_id');
 
-		if ($userId !== null && $userId !== '') {
-			$qb->andWhere($qb->expr()->eq('ownership', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)))
-			->orWhere($shareViewQuery->expr()->in('v.id', $qb->createFunction($shareViewQuery->getSQL()), IQueryBuilder::PARAM_INT_ARRAY))
-			->orWhere($shareTableQuery->expr()->in('v.table_id', $qb->createFunction($shareTableQuery->getSQL()), IQueryBuilder::PARAM_INT_ARRAY));
+		if ($userId) {
+			$qb->where($qb->expr()->eq('ownership', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)))
+			->orWhere($shareQueryViewsSharedViaUser->expr()->in('v.id', $qb->createFunction($shareQueryViewsSharedViaUser->getSQL()), IQueryBuilder::PARAM_INT_ARRAY));
+			if($userGroups) {
+				$qb->orWhere($shareQueryViewsSharedViaGroup->expr()->in('v.id', $qb->createFunction($shareQueryViewsSharedViaGroup->getSQL()), IQueryBuilder::PARAM_INT_ARRAY));
+			}
 		}
 
 		if ($term) {