From 602ac4cfdfd37ad899f4052ed48d799e763af4d4 Mon Sep 17 00:00:00 2001 From: Vitor George Date: Mon, 19 Dec 2022 15:51:51 +0000 Subject: [PATCH 01/24] Add function to generate zero-padded numbers for consistent sorting --- cypress/e2e/auth.cy.js | 2 +- cypress/e2e/profile.cy.js | 6 +++--- cypress/e2e/team-invitations.cy.js | 2 +- cypress/e2e/teams.cy.js | 2 +- src/lib/utils.js | 23 ++++++++++++++++++++--- src/models/team.js | 2 +- 6 files changed, 27 insertions(+), 10 deletions(-) diff --git a/cypress/e2e/auth.cy.js b/cypress/e2e/auth.cy.js index 1196798b..e6972058 100644 --- a/cypress/e2e/auth.cy.js +++ b/cypress/e2e/auth.cy.js @@ -30,7 +30,7 @@ describe('Check protected routes', () => { // Authorized visit, should redirect to sign in cy.login({ id: 1, - display_name: 'User 1', + display_name: 'User 001', }) cy.visit(testRoute) cy.get('body').should('not.contain', 'Sign in') diff --git a/cypress/e2e/profile.cy.js b/cypress/e2e/profile.cy.js index ea5e841a..411083b9 100644 --- a/cypress/e2e/profile.cy.js +++ b/cypress/e2e/profile.cy.js @@ -2,17 +2,17 @@ const { generateSequenceArray } = require('../../src/lib/utils') const user1 = { id: 1, - display_name: 'User 1', + display_name: 'User 001', } const user2 = { id: 2, - display_name: 'User 2', + display_name: 'User 002', } const user3 = { id: 3, - display_name: 'User 3', + display_name: 'User 003', } /** diff --git a/cypress/e2e/team-invitations.cy.js b/cypress/e2e/team-invitations.cy.js index 8b9bc30e..6a743627 100644 --- a/cypress/e2e/team-invitations.cy.js +++ b/cypress/e2e/team-invitations.cy.js @@ -60,7 +60,7 @@ describe('Team invitation page', () => { it('Valid invitation - user is authenticated', () => { cy.login({ id: 1, - display_name: 'User 1', + display_name: 'User 001', }) cy.visit( `/teams/${validInvitation.teamId}/invitations/${validInvitation.uuid}` diff --git a/cypress/e2e/teams.cy.js b/cypress/e2e/teams.cy.js index a6ffdf4c..39b30e22 100644 --- a/cypress/e2e/teams.cy.js +++ b/cypress/e2e/teams.cy.js @@ -21,7 +21,7 @@ describe('Teams page', () => { // Signed in team member cy.login({ id: 1, - display_name: 'User 1', + display_name: 'User 001', }) cy.visit('/teams/1') cy.get('body').should('contain', 'Team Members') diff --git a/src/lib/utils.js b/src/lib/utils.js index a0bfe1c6..08f4e80c 100644 --- a/src/lib/utils.js +++ b/src/lib/utils.js @@ -1,4 +1,4 @@ -export function getRandomColor() { +function getRandomColor() { var letters = '0123456789ABCDEF' var color = '#' for (var i = 0; i < 6; i++) { @@ -13,7 +13,7 @@ export function getRandomColor() { * @param {Number or String} timestamp * @returns */ -export function toDateString(timestamp) { +function toDateString(timestamp) { const dateFormat = new Intl.DateTimeFormat(navigator.language).format return dateFormat(new Date(timestamp)) } @@ -25,6 +25,23 @@ export function toDateString(timestamp) { * @param {Number} initialValue Initial value * @returns */ -export function generateSequenceArray(length, initialValue = 0) { +function generateSequenceArray(length, initialValue = 0) { return Array.from({ length }, (_, i) => i + initialValue) } + +/** + * Add leading zeroes to a number + * @param {*} n the number + * @param {*} width final length + * @returns zero-padded number + */ +function addZeroPadding(n, width) { + return String(n).padStart(width, '0') +} + +module.exports = { + getRandomColor, + toDateString, + generateSequenceArray, + addZeroPadding, +} diff --git a/src/models/team.js b/src/models/team.js index 53c58f4e..77ffc3ec 100644 --- a/src/models/team.js +++ b/src/models/team.js @@ -71,7 +71,7 @@ async function resolveMemberNames(ids) { if (process.env.TESTING === 'true') { return ids.map((id) => ({ id, - name: `User ${id}`, + name: `User ${addZeroPadding(id, 3)}`, // use zero-padded number for consistent table sorting image: `https://via.placeholder.com/150`, })) } From 88889682a4e68f6cf93b87cc9c628090363f34b9 Mon Sep 17 00:00:00 2001 From: Vitor George Date: Mon, 19 Dec 2022 15:57:45 +0000 Subject: [PATCH 02/24] Refactor and connect org team members table at org page --- app/manage/index.js | 12 -- app/manage/organizations.js | 59 ---------- app/manage/permissions/index.js | 1 - app/manage/permissions/view-org-members.js | 27 ----- cypress.config.js | 13 ++- cypress/e2e/organizations.cy.js | 107 +++++++++++++++--- next.config.js | 1 + src/components/tables/users.js | 58 ++++++++++ src/lib/org-api.js | 38 ------- src/middlewares/can/view-org-members.js | 6 +- src/models/organization.js | 55 ++++++--- src/models/team.js | 6 +- .../api/organizations/[orgId]/members.js | 60 ++++++++++ src/pages/organizations/[id]/index.js | 102 ++--------------- 14 files changed, 278 insertions(+), 267 deletions(-) delete mode 100644 app/manage/permissions/view-org-members.js create mode 100644 src/components/tables/users.js create mode 100644 src/pages/api/organizations/[orgId]/members.js diff --git a/app/manage/index.js b/app/manage/index.js index df584517..fe9b58a7 100644 --- a/app/manage/index.js +++ b/app/manage/index.js @@ -27,9 +27,7 @@ const { removeOwner, addManager, removeManager, - getOrgMembers, listMyOrgs, - getOrgStaff, } = require('./organizations') const { @@ -123,16 +121,6 @@ function manageRouter(handler) { handler.get('/api/organizations/:id', can('public:authenticated'), getOrg) handler.put('/api/organizations/:id', can('organization:edit'), updateOrg) handler.delete('/api/organizations/:id', can('organization:edit'), destroyOrg) - handler.get( - '/api/organizations/:id/staff', - can('organization:view-members'), - getOrgStaff - ) - handler.get( - '/api/organizations/:id/members', - can('organization:view-members'), - getOrgMembers - ) handler.put( '/api/organizations/:id/addOwner/:osmId', diff --git a/app/manage/organizations.js b/app/manage/organizations.js index e0c9402a..e7a00c2c 100644 --- a/app/manage/organizations.js +++ b/app/manage/organizations.js @@ -1,6 +1,4 @@ const organization = require('../../src/models/organization') -const team = require('../../src/models/team') -const { map, prop } = require('ramda') const Boom = require('@hapi/boom') /** @@ -54,61 +52,6 @@ async function getOrg(req, reply) { reply.send({ ...data, isMemberOfOrg }) } -/** - * Get an organization's staff - * Requires id of organization - */ -async function getOrgStaff(req, reply) { - const { id } = req.params - - if (!id) { - throw Boom.badRequest('organization id is required') - } - - try { - let [owners, managers] = await Promise.all([ - organization.getOwners(id), - organization.getManagers(id), - ]) - const ownerIds = map(prop('osm_id'), owners) - const managerIds = map(prop('osm_id'), managers) - if (ownerIds.length > 0) { - owners = await team.resolveMemberNames(ownerIds) - } - if (managerIds.length > 0) { - managers = await team.resolveMemberNames(managerIds) - } - - reply.send({ owners, managers }) - } catch (err) { - console.log(err) - throw Boom.badRequest(err.message) - } -} - -async function getOrgMembers(req, reply) { - const { id } = req.params - - if (!id) { - throw Boom.badRequest('organization id is required') - } - - let { page } = req.query - - try { - let members = await organization.getMembers(id, page) - const memberIds = map(prop('osm_id'), members) - if (memberIds.length > 0) { - members = await team.resolveMemberNames(memberIds) - } - - reply.send({ members, page }) - } catch (err) { - console.log(err) - throw Boom.badRequest(err.message) - } -} - /** * Update an organization * Requires the id of the organization to modify @@ -246,6 +189,4 @@ module.exports = { addManager, removeManager, listMyOrgs, - getOrgStaff, - getOrgMembers, } diff --git a/app/manage/permissions/index.js b/app/manage/permissions/index.js index 92551760..2ba3c766 100644 --- a/app/manage/permissions/index.js +++ b/app/manage/permissions/index.js @@ -24,7 +24,6 @@ const teamPermissions = { const organizationPermissions = { 'organization:edit': require('./edit-org'), 'organization:member': require('./member-org'), - 'organization:view-members': require('./view-org-members'), 'organization:view-team-keys': require('./view-org-team-keys'), } diff --git a/app/manage/permissions/view-org-members.js b/app/manage/permissions/view-org-members.js deleted file mode 100644 index 042146b9..00000000 --- a/app/manage/permissions/view-org-members.js +++ /dev/null @@ -1,27 +0,0 @@ -const { - isPublic, - isMemberOrStaff, -} = require('../../../src/models/organization') - -/** - * org:view-members - * - * To view an org's members, the org needs to be either public - * or the user should be a member of the org - * - * @param {string} uid user id - * @param {Object} params request parameters - * @returns {boolean} can the request go through? - */ -async function viewOrgMembers(uid, { id }) { - try { - const publicOrg = await isPublic(id) - if (publicOrg) return publicOrg - return await isMemberOrStaff(id, uid) - } catch (e) { - console.error(e) - return false - } -} - -module.exports = viewOrgMembers diff --git a/cypress.config.js b/cypress.config.js index 1614e055..8c3bbcf3 100644 --- a/cypress.config.js +++ b/cypress.config.js @@ -52,6 +52,13 @@ module.exports = defineConfig({ } return null }, + 'db:seed:add-members-to-team': async ({ teamId, members }) => { + for (let i = 0; i < members.length; i++) { + const member = members[i] + await Team.addMember(teamId, member.id) + } + return null + }, 'db:seed:team-invitations': async (teamInvitations) => { return Promise.all(teamInvitations.map(TeamInvitation.create)) }, @@ -62,7 +69,11 @@ module.exports = defineConfig({ ) ) }, - 'db:seed:organization-teams': async ({ orgId, teams, managerId }) => { + 'db:seed:add-organization-teams': async ({ + orgId, + teams, + managerId, + }) => { return Promise.all( teams.map((team) => Organization.createOrgTeam(orgId, pick(['name'], team), managerId) diff --git a/cypress/e2e/organizations.cy.js b/cypress/e2e/organizations.cy.js index 41169f21..9e135184 100644 --- a/cypress/e2e/organizations.cy.js +++ b/cypress/e2e/organizations.cy.js @@ -1,21 +1,44 @@ -const { generateSequenceArray } = require('../../src/lib/utils') +const { generateSequenceArray, addZeroPadding } = require('../../src/lib/utils') +// Owner user const user1 = { id: 1, - display_name: 'User 1', + display_name: 'User 001', } +// Organization meta const org1 = { id: 1, name: 'My org', ownerId: user1.id, } +// Generate org teams const ORG_TEAMS_COUNT = 35 +const orgTeams = generateSequenceArray(ORG_TEAMS_COUNT, 1).map((i) => ({ + id: i, + name: `Org 1 Team ${addZeroPadding(i, 3)}`, +})) + +// Keep first three org teams +const [orgTeam1, orgTeam2, orgTeam3] = orgTeams + +// Generate 10 users for org team 1, from using id range starting at 100 +const orgTeam1Members = generateSequenceArray(10, 100).map((i) => ({ + id: i, + name: `User ${addZeroPadding(i, 3)}`, +})) -const teams = generateSequenceArray(ORG_TEAMS_COUNT, 1).map((i) => ({ +// Generate 20 users for org team 2, from using id range starting at 200 +const orgTeam2Members = generateSequenceArray(20, 200).map((i) => ({ id: i, - name: `Team ${i}`, + name: `User ${addZeroPadding(i, 3)}`, +})) + +// Generate 15 users for org team 3, from using id range starting at 300 +const orgTeam3Members = generateSequenceArray(15, 300).map((i) => ({ + id: i, + name: `User ${addZeroPadding(i, 3)}`, })) describe('Organization page', () => { @@ -35,36 +58,53 @@ describe('Organization page', () => { cy.get('[data-cy=org-teams-table-pagination]').should('not.exist') }) - it('Display paginated list of teams', () => { + it.only('Display paginated list of teams', () => { cy.login(user1) - // Seed org teams - cy.task('db:seed:organization-teams', { + // Add org teams + cy.task('db:seed:add-organization-teams', { orgId: org1.id, - teams, + teams: orgTeams, managerId: user1.id, }) + // Add members to org team 1 + cy.task('db:seed:add-members-to-team', { + teamId: orgTeam1.id, + members: orgTeam1Members, + }) + + // Add members to org team 2 + cy.task('db:seed:add-members-to-team', { + teamId: orgTeam2.id, + members: orgTeam2Members, + }) + + // Add members to org team 5 + cy.task('db:seed:add-members-to-team', { + teamId: orgTeam3.id, + members: orgTeam3Members, + }) + // Check state when teams are available cy.visit('/organizations/1') - cy.get('[data-cy=org-teams-table]').contains('Team 10') + cy.get('[data-cy=org-teams-table]').contains('Org 1 Team 010') - // Click last page button + // Verify index, then click on last page button cy.get('[data-cy=org-teams-table-pagination]').within(() => { + cy.contains('Showing 1-10 of 35') cy.get('[data-cy=last-page-button]').click() }) - // Last item is present - cy.get('[data-cy=org-teams-table]').contains('Team 9') - // Click page 2 button cy.get('[data-cy=org-teams-table-pagination]').within(() => { cy.get('[data-cy=page-2-button]').click() + cy.contains('Showing 11-20 of 35') }) // Item from page 2 is present - cy.get('[data-cy=org-teams-table]').contains('Team 2') + cy.get('[data-cy=org-teams-table]').contains('Org 1 Team 012') // Click next page button cy.get('[data-cy=org-teams-table-pagination]').within(() => { @@ -72,7 +112,7 @@ describe('Organization page', () => { }) // Item from page 3 is present - cy.get('[data-cy=org-teams-table]').contains('Team 3') + cy.get('[data-cy=org-teams-table]').contains('Org 1 Team 028') // Click previous page button cy.get('[data-cy=org-teams-table-pagination]').within(() => { @@ -80,6 +120,41 @@ describe('Organization page', () => { }) // Item from page 2 is present - cy.get('[data-cy=org-teams-table]').contains('Team 2') + cy.get('[data-cy=org-teams-table]').contains('Org 1 Team 015') + + // ORG TEAM MEMBERS + cy.get('[data-cy=org-members-table]').should('exist') + + // Click last page button + cy.get('[data-cy=org-members-table-pagination]').within(() => { + cy.contains('Showing 1-10 of 46') + cy.get('[data-cy=last-page-button]').click() + }) + + // Last item is present + cy.get('[data-cy=org-members-table]').contains('User 314') + + // Click page 2 button + cy.get('[data-cy=org-members-table-pagination]').within(() => { + cy.get('[data-cy=page-2-button]').click() + }) + + // Item from page 2 is present + cy.get('[data-cy=org-members-table]').contains('User 203') + + // Click next page button + cy.get('[data-cy=org-members-table-pagination]').within(() => { + cy.contains('Showing 11-20 of 46') + cy.get('[data-cy=next-page-button]').click() + }) + + // On page 3, click previous page button + cy.get('[data-cy=org-members-table-pagination]').within(() => { + cy.contains('Showing 21-30 of 46') + cy.get('[data-cy=previous-page-button]').click() + }) + + // Item from page 2 is present + cy.get('[data-cy=org-members-table]').contains('User 207') }) }) diff --git a/next.config.js b/next.config.js index 91cc6b13..8eb9d347 100644 --- a/next.config.js +++ b/next.config.js @@ -4,6 +4,7 @@ const vercelUrl = module.exports = { serverRuntimeConfig: { + DEFAULT_PAGE_SIZE: 10, NODE_ENV: process.env.NODE_ENV || 'development', OSM_DOMAIN: process.env.OSM_DOMAIN || 'https://www.openstreetmap.org', OSM_API: diff --git a/src/components/tables/users.js b/src/components/tables/users.js new file mode 100644 index 00000000..5e2073bc --- /dev/null +++ b/src/components/tables/users.js @@ -0,0 +1,58 @@ +import T from 'prop-types' +import Table from './table' +import { useFetchList } from '../../hooks/use-fetch-list' +import { useState } from 'react' +import Pagination from '../pagination' + +function UsersTable({ type, orgId, onRowClick }) { + const [page, setPage] = useState(1) + + let apiBasePath + let emptyMessage + + switch (type) { + case 'org-members': + apiBasePath = `/organizations/${orgId}/members` + emptyMessage = 'No members yet.' + break + case 'org-staff': + apiBasePath = `/organizations/${orgId}/staff` + emptyMessage = 'No staff found.' + break + default: + break + } + + const { + result: { data, pagination }, + isLoading, + } = useFetchList(`${apiBasePath}?page=${page}`) + + const columns = [{ key: 'name' }, { key: 'id', label: 'OSM ID' }] + + return ( + <> + + {pagination?.total > 0 && ( + + )} + + ) +} + +UsersTable.propTypes = { + type: T.oneOf(['org-members', 'org-staff']).isRequired, + orgId: T.number, +} + +export default UsersTable diff --git a/src/lib/org-api.js b/src/lib/org-api.js index a1489d8a..3b9b65d3 100644 --- a/src/lib/org-api.js +++ b/src/lib/org-api.js @@ -56,44 +56,6 @@ export async function getOrg(id) { } } -/** - * getOrgStaff - * get org staff - * @param {integer} id - */ -export async function getOrgStaff(id) { - let res = await fetch(join(ORG_URL, `${id}`, 'staff')) - if (res.status === 200) { - return res.json() - } - if (res.status === 401) { - return { managers: [], owners: [] } - } else { - const err = new Error('could not retrieve organization') - err.status = res.status - throw err - } -} -/** - * getMembers - * get org members (paginated) - * @param {integer} id - * @param {integer} page - */ -export async function getMembers(id, page) { - let res = await fetch(join(ORG_URL, `${id}`, 'members', `?page=${page}`)) - if (res.status === 200) { - return res.json() - } - if (res.status === 401) { - return { members: [], page } - } else { - const err = new Error('could not retrieve organization') - err.status = res.status - throw err - } -} - /** * updateOrg * @param {integer} id id of organization diff --git a/src/middlewares/can/view-org-members.js b/src/middlewares/can/view-org-members.js index 13a2dde2..b53b00bb 100644 --- a/src/middlewares/can/view-org-members.js +++ b/src/middlewares/can/view-org-members.js @@ -12,7 +12,11 @@ export default async function canViewOrgMembers(req, res, next) { const { orgId } = req.query const { user_id: userId } = req.session - if ((await isPublic(orgId)) || (await isMemberOrStaff(orgId, userId))) { + if (await isPublic(orgId)) { + // Can view if org is public + return next() + } else if (await isMemberOrStaff(orgId, userId)) { + // Can view if is member or staff return next() } else { throw Boom.unauthorized() diff --git a/src/models/organization.js b/src/models/organization.js index 64a2c824..c558fa94 100644 --- a/src/models/organization.js +++ b/src/models/organization.js @@ -3,6 +3,10 @@ const team = require('./team') const { map, prop, includes, has, isNil } = require('ramda') const { unpack, PropertyRequiredError } = require('../../app/lib/utils') +const { serverRuntimeConfig } = require('../../next.config') + +const { DEFAULT_PAGE_SIZE } = serverRuntimeConfig + // Organization attributes (without profile) const orgAttributes = [ 'id', @@ -244,34 +248,57 @@ async function createOrgTeam(organizationId, data, osmId) { * Get all members and associated teams of an organization * We get all members of all associated teams with this organization * @param {int} organizationId - organization id + * @param {object} options - pagination params + * */ -async function getMembers(organizationId, page) { - if (!organizationId) throw new PropertyRequiredError('organization id') +async function getMembers(organizationId, options) { + const currentPage = options?.page || 1 - const subquery = db('organization_team') + // Sub-query for all org teams + const allOrgTeamsQuery = db('organization_team') .select('team_id') .where('organization_id', organizationId) - let query = db('member') - .select(db.raw('array_agg(team_id) as teams, osm_id')) - .where('team_id', 'in', subquery) + + // Get list of member of org teams, paginated + const result = await db('member') + .select('osm_id') + .where('team_id', 'in', allOrgTeamsQuery) .groupBy('osm_id') + .orderBy('osm_id') + .paginate({ + isLengthAware: true, + currentPage, + perPage: DEFAULT_PAGE_SIZE, + }) - if (page) { - query = query.limit(50).offset(page * 20) + // Resolver member names + const data = await team.resolveMemberNames(result.data.map((m) => m.osm_id)) + + return { + ...result, + data, } - return query } /** - * Checks if an osmId is part of an organization members + * Check if user is part of an organization team * @param {int} organizationId - organization id * @param {int} osmId - id of member we are testing */ async function isMember(organizationId, osmId) { - if (!osmId) throw new PropertyRequiredError('osm id') - const members = await getMembers(organizationId) - const memberIds = members.map(prop('osm_id')) - return includes(Number(osmId), map(Number, memberIds)) + // Sub-query for all org teams + const orgTeamsQuery = db('organization_team') + .select('team_id') + .where('organization_id', organizationId) + + // Check if user is member of at least one org team + const memberships = await db('member') + .select('osm_id') + .where('team_id', 'in', orgTeamsQuery) + .where('osm_id', osmId) + .limit(1) + + return memberships.length > 0 } /** diff --git a/src/models/team.js b/src/models/team.js index 77ffc3ec..77be0091 100644 --- a/src/models/team.js +++ b/src/models/team.js @@ -5,10 +5,10 @@ const xml2js = require('xml2js') const { unpack } = require('../../app/lib/utils') const { prop, isEmpty } = require('ramda') const request = require('request-promise-native') +const { addZeroPadding } = require('../lib/utils') const { serverRuntimeConfig } = require('../../next.config') - -const DEFAULT_LIMIT = 10 +const { DEFAULT_PAGE_SIZE } = serverRuntimeConfig /** * @swagger @@ -211,7 +211,7 @@ async function list(options = {}) { : query.paginate({ isLengthAware: true, currentPage: page || 1, - perPage: DEFAULT_LIMIT, + perPage: DEFAULT_PAGE_SIZE, }) } diff --git a/src/pages/api/organizations/[orgId]/members.js b/src/pages/api/organizations/[orgId]/members.js new file mode 100644 index 00000000..b3791302 --- /dev/null +++ b/src/pages/api/organizations/[orgId]/members.js @@ -0,0 +1,60 @@ +import { createBaseHandler } from '../../../../middlewares/base-handler' +import { validate } from '../../../../middlewares/validation' +import Organization from '../../../../models/organization' +import Team from '../../../../models/team' +import * as Yup from 'yup' +import canViewOrgMembers from '../../../../middlewares/can/view-org-members' +import { map, prop } from 'ramda' + +const handler = createBaseHandler() + +/** + * @swagger + * /organizations/{id}/members: + * get: + * summary: Get list of members of an organization + * tags: + * - organizations + * parameters: + * - in: path + * name: id + * required: true + * description: Organization id + * schema: + * type: integer + * responses: + * 200: + * description: A list of members + * content: + * application/json: + * schema: + * type: object + * properties: + * pagination: + * $ref: '#/components/schemas/Pagination' + * data: + * $ref: '#/components/schemas/ArrayOfMembers' + */ +handler.get( + canViewOrgMembers, + validate({ + query: Yup.object({ + orgId: Yup.number().required().positive().integer(), + page: Yup.number().min(0).integer(), + }).required(), + }), + async function (req, res) { + const { orgId, page } = req.query + + let members = await Organization.getMembers(orgId, { page, paginate: true }) + + const memberIds = map(prop('osm_id'), members) + if (memberIds.length > 0) { + members = await Team.resolveMemberNames(memberIds) + } + + return res.send(members) + } +) + +export default handler diff --git a/src/pages/organizations/[id]/index.js b/src/pages/organizations/[id]/index.js index 002852dd..ad41cc92 100644 --- a/src/pages/organizations/[id]/index.js +++ b/src/pages/organizations/[id]/index.js @@ -2,8 +2,6 @@ import React, { Component } from 'react' import Router, { withRouter } from 'next/router' import { getOrg, - getOrgStaff, - getMembers, addManager, removeManager, addOwner, @@ -20,11 +18,12 @@ import SvgSquare from '../../../components/svg-square' import Button from '../../../components/button' import Modal from 'react-modal' import ProfileModal from '../../../components/profile-modal' -import { assoc, propEq, find, contains, prop, map } from 'ramda' +import { contains, prop, map } from 'ramda' import APIClient from '../../../lib/api-client' import join from 'url-join' import { getSession } from 'next-auth/react' import TeamsTable from '../../../components/tables/teams' +import UsersTable from '../../../components/tables/users' const URL = process.env.APP_URL @@ -76,9 +75,9 @@ class Organization extends Component { async componentDidMount() { this.setState({ session: await getSession() }) await this.getOrg() - await this.getOrgStaff() + // await this.getOrgStaff() await this.getBadges() - return this.getMembers(0) + // return this.getMembers(0) } async openProfileModal(user) { @@ -115,54 +114,6 @@ class Organization extends Component { }) } - async getOrgStaff() { - const { id } = this.props - try { - let { managers, owners } = await getOrgStaff(id) - this.setState({ - managers, - owners, - }) - } catch (e) { - console.error(e) - this.setState({ - error: e, - managers: [], - owners: [], - loading: false, - }) - } - } - - async getMembers(currentPage) { - const { id } = this.props - try { - let { members, page } = await getMembers(id, currentPage) - this.setState({ - members, - page: Number(page), - loading: false, - }) - } catch (e) { - console.error(e) - this.setState({ - error: e, - org: null, - loading: false, - }) - } - } - - async getNextPage() { - this.setState({ loading: true }) - await this.getMembers(this.state.page + 1) - } - - async getPrevPage() { - this.setState({ loading: true }) - await this.getMembers(this.state.page - 1) - } - async getOrg() { const { id } = this.props try { @@ -180,29 +131,6 @@ class Organization extends Component { } } - renderStaff(owners, managers) { - const columns = [{ key: 'name' }, { key: 'id' }, { key: 'role' }] - const ownerRows = owners.map(assoc('role', 'owner')) - const managerRows = managers.map(assoc('role', 'manager')) - let allRows = ownerRows - managerRows.forEach((row) => { - if (!find(propEq('id', row.id))(ownerRows)) { - ownerRows.push(row) - } - }) - - return ( -
this.openProfileModal(row)} - /> - ) - } - async getBadges() { try { const { id: orgId } = this.props @@ -262,24 +190,8 @@ class Organization extends Component { ) : null } - renderMembers(memberRows) { - const columns = [{ key: 'name' }, { key: 'id' }] - return ( -
this.openProfileModal(row)} - /> - ) - } - render() { - const { org, members, managers, owners, error } = this.state + const { org, managers, owners, error } = this.state if (!org) return null const userId = parseInt(this.state.session.user_id) @@ -403,7 +315,7 @@ class Organization extends Component { - {this.renderStaff(owners, managers)} + ) : (
@@ -437,7 +349,7 @@ class Organization extends Component {
- {this.renderMembers(members)} + ) : (
From 873d6d909f530b21182f249656ed5ea8dd75ba74 Mon Sep 17 00:00:00 2001 From: Vitor George Date: Mon, 19 Dec 2022 15:58:09 +0000 Subject: [PATCH 03/24] Add log level instructions to README --- README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 418529cc..28a5659f 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ Visit your [OpenStreetMap settings](https://www.openstreetmap.org/account/edit) Example: -![OSM Client App](oauth2-osm-client-app.png "OAuth 2 page at OSM Website") +![OSM Client App](oauth2-osm-client-app.png 'OAuth 2 page at OSM Website') Create an `.env.local` file and add environment variables `OSM_CONSUMER_KEY` and `OSM_CONSUMER_SECRET` obtained at OAuth2 page at OpenStreetMap website. The `.env.local` file should be like the following: @@ -44,7 +44,9 @@ Start development server: yarn dev + ✨ You can now login to the app at http://127.0.0.1:3000 + ## Testing @@ -61,6 +63,8 @@ To open Cypress dashboard for interactive development: yarn e2e:dev +By default, logging level in testing environment is set to 'silent'. Please refer to pino docs for the full [list of log levels](https://getpino.io/#/docs/api?id=level-string). + ## API The API docs can be accessed at . From d7bb07382a495fbd04caeacc26b9a80a5cfcc2ea Mon Sep 17 00:00:00 2001 From: Vitor George Date: Mon, 19 Dec 2022 17:24:21 +0000 Subject: [PATCH 04/24] Populate staff table --- src/components/tables/users.js | 9 +++- src/models/organization.js | 26 ++++++--- src/pages/api/organizations/[orgId]/staff.js | 56 ++++++++++++++++++++ 3 files changed, 81 insertions(+), 10 deletions(-) create mode 100644 src/pages/api/organizations/[orgId]/staff.js diff --git a/src/components/tables/users.js b/src/components/tables/users.js index 5e2073bc..e8e328ed 100644 --- a/src/components/tables/users.js +++ b/src/components/tables/users.js @@ -9,15 +9,22 @@ function UsersTable({ type, orgId, onRowClick }) { let apiBasePath let emptyMessage + let columns switch (type) { case 'org-members': apiBasePath = `/organizations/${orgId}/members` emptyMessage = 'No members yet.' + columns = [{ key: 'name' }, { key: 'id', label: 'OSM ID' }] break case 'org-staff': apiBasePath = `/organizations/${orgId}/staff` emptyMessage = 'No staff found.' + columns = [ + { key: 'name' }, + { key: 'id', label: 'OSM ID' }, + { key: 'type' }, + ] break default: break @@ -28,8 +35,6 @@ function UsersTable({ type, orgId, onRowClick }) { isLoading, } = useFetchList(`${apiBasePath}?page=${page}`) - const columns = [{ key: 'name' }, { key: 'id', label: 'OSM ID' }] - return ( <>
m.osm_id)) + + return { + ...result, + data, + } } /** diff --git a/src/pages/api/organizations/[orgId]/staff.js b/src/pages/api/organizations/[orgId]/staff.js new file mode 100644 index 00000000..81240555 --- /dev/null +++ b/src/pages/api/organizations/[orgId]/staff.js @@ -0,0 +1,56 @@ +import { createBaseHandler } from '../../../../middlewares/base-handler' +import { validate } from '../../../../middlewares/validation' +import Organization from '../../../../models/organization' +import * as Yup from 'yup' +import canViewOrgMembers from '../../../../middlewares/can/view-org-members' + +const handler = createBaseHandler() + +/** + * @swagger + * /organizations/{id}/staff: + * get: + * summary: Get list of staff of an organization + * tags: + * - organizations + * parameters: + * - in: path + * name: id + * required: true + * description: Organization id + * schema: + * type: integer + * responses: + * 200: + * description: A list of members + * content: + * application/json: + * schema: + * type: object + * properties: + * pagination: + * $ref: '#/components/schemas/Pagination' + * data: + * $ref: '#/components/schemas/ArrayOfStaff' + */ +handler.get( + canViewOrgMembers, + validate({ + query: Yup.object({ + orgId: Yup.number().required().positive().integer(), + page: Yup.number().min(0).integer(), + }).required(), + }), + async function (req, res) { + const { orgId, page } = req.query + + const staff = await Organization.getOrgStaff({ + page, + organizationId: orgId, + }) + + return res.send(staff) + } +) + +export default handler From 54c00dd9db0eb43f0d2bff6598bb99682da61e0d Mon Sep 17 00:00:00 2001 From: Vitor George Date: Tue, 20 Dec 2022 12:13:17 +0000 Subject: [PATCH 05/24] Simplify Organization.getOrgStaff() method --- src/models/organization.js | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/src/models/organization.js b/src/models/organization.js index c63691e0..b2b6d911 100644 --- a/src/models/organization.js +++ b/src/models/organization.js @@ -382,35 +382,23 @@ async function isManager(organizationId, osmId) { * @param {Object} options.osmId - filter by osm id */ async function getOrgStaff(options = {}) { - console.log('getOrgStaff') + // Get owners let ownerQuery = db('organization_owner') .select(db.raw("organization_id, osm_id, 'owner' as type")) - .join( - 'organization', - 'organization.id', - 'organization_owner.organization_id' - ) + .where('organization_id', options.organizationId) + // Get managers that are not owners let managerQuery = db('organization_manager') .select(db.raw("organization_id, osm_id, 'manager' as type")) - .join( - 'organization', - 'organization.id', - 'organization_manager.organization_id' - ) - - if (options.organizationId) { - ownerQuery = ownerQuery.where('organization.id', options.organizationId) - managerQuery = managerQuery.where('organization.id', options.organizationId) - } - if (options.osmId) { - ownerQuery = ownerQuery.where('organization_owner.osm_id', options.osmId) - managerQuery = managerQuery.where( - 'organization_manager.osm_id', - options.osmId + .where('organization_id', options.organizationId) + .whereNotIn( + 'osm_id', + db('organization_owner') + .select('osm_id') + .where('organization_id', options.organizationId) ) - } + // Execute query with paginations const result = await ownerQuery.unionAll(managerQuery).paginate({ isLengthAware: true, currentPage: options.page || 1, From 86beef459eba21a44c15d63c9a57b314e8d03f80 Mon Sep 17 00:00:00 2001 From: Vitor George Date: Tue, 20 Dec 2022 12:43:26 +0000 Subject: [PATCH 06/24] Populate staff table --- cypress.config.js | 7 ++++ cypress/e2e/organizations.cy.js | 39 ++++++++++++++++---- src/models/organization.js | 28 +++++++++----- src/pages/api/organizations/[orgId]/staff.js | 5 +-- 4 files changed, 60 insertions(+), 19 deletions(-) diff --git a/cypress.config.js b/cypress.config.js index 8c3bbcf3..042804c9 100644 --- a/cypress.config.js +++ b/cypress.config.js @@ -80,6 +80,13 @@ module.exports = defineConfig({ ) ) }, + 'db:seed:add-organization-managers': async ({ orgId, managerIds }) => { + for (let i = 0; i < managerIds.length; i++) { + const managerId = managerIds[i] + await Organization.addManager(orgId, managerId) + } + return null + }, }) }, }, diff --git a/cypress/e2e/organizations.cy.js b/cypress/e2e/organizations.cy.js index 9e135184..729ddc7c 100644 --- a/cypress/e2e/organizations.cy.js +++ b/cypress/e2e/organizations.cy.js @@ -1,10 +1,11 @@ const { generateSequenceArray, addZeroPadding } = require('../../src/lib/utils') -// Owner user -const user1 = { - id: 1, - display_name: 'User 001', -} +const orgStaff = generateSequenceArray(7, 1).map((i) => ({ + id: i, + name: `User ${addZeroPadding(i, 3)}`, +})) + +const [user1, user2, user3] = orgStaff // Organization meta const org1 = { @@ -58,7 +59,26 @@ describe('Organization page', () => { cy.get('[data-cy=org-teams-table-pagination]').should('not.exist') }) - it.only('Display paginated list of teams', () => { + it('Organization staff table is populated and paginated', () => { + cy.login(user1) + + // Add org staff + cy.task('db:seed:add-organization-managers', { + orgId: org1.id, + managerIds: [user2.id, user3.id], + }) + + // Check state when teams are available + cy.visit('/organizations/1') + + // Org staff table is populated + cy.get('[data-cy=org-staff-table]').contains('User 002') + cy.get('[data-cy=org-staff-table-pagination]').within(() => { + cy.contains('Showing 1-3 of 3') + }) + }) + + it('Organization teams and members tables are populated and paginated', () => { cy.login(user1) // Add org teams @@ -89,6 +109,9 @@ describe('Organization page', () => { // Check state when teams are available cy.visit('/organizations/1') + /** + * ORG TEAMS + */ cy.get('[data-cy=org-teams-table]').contains('Org 1 Team 010') // Verify index, then click on last page button @@ -122,7 +145,9 @@ describe('Organization page', () => { // Item from page 2 is present cy.get('[data-cy=org-teams-table]').contains('Org 1 Team 015') - // ORG TEAM MEMBERS + /** + * ORG TEAM MEMBERS + */ cy.get('[data-cy=org-members-table]').should('exist') // Click last page button diff --git a/src/models/organization.js b/src/models/organization.js index b2b6d911..b8f5b0f8 100644 --- a/src/models/organization.js +++ b/src/models/organization.js @@ -1,6 +1,6 @@ const db = require('../lib/db') const team = require('./team') -const { map, prop, includes, has, isNil } = require('ramda') +const { map, prop, includes, has, isNil, find, propEq } = require('ramda') const { unpack, PropertyRequiredError } = require('../../app/lib/utils') const { serverRuntimeConfig } = require('../../next.config') @@ -381,36 +381,46 @@ async function isManager(organizationId, osmId) { * @param {Object} options.organizationId - filter by organization * @param {Object} options.osmId - filter by osm id */ -async function getOrgStaff(options = {}) { +async function getOrgStaff(organizationId, options = {}) { // Get owners let ownerQuery = db('organization_owner') .select(db.raw("organization_id, osm_id, 'owner' as type")) - .where('organization_id', options.organizationId) + .where('organization_id', organizationId) // Get managers that are not owners let managerQuery = db('organization_manager') .select(db.raw("organization_id, osm_id, 'manager' as type")) - .where('organization_id', options.organizationId) + .where('organization_id', organizationId) .whereNotIn( 'osm_id', db('organization_owner') .select('osm_id') - .where('organization_id', options.organizationId) + .where('organization_id', organizationId) ) - // Execute query with paginations + // Execute query with pagination const result = await ownerQuery.unionAll(managerQuery).paginate({ isLengthAware: true, currentPage: options.page || 1, perPage: DEFAULT_PAGE_SIZE, }) - // Resolver member names - const data = await team.resolveMemberNames(result.data.map((m) => m.osm_id)) + // Get osm user meta from ids + const osmUsers = await team.resolveMemberNames( + result.data.map((m) => m.osm_id) + ) return { ...result, - data, + // Apply OSM display names to results + data: result.data.map((u) => { + const osmUser = find(propEq('id', u.osm_id))(osmUsers) + return { + ...u, + id: u.osm_id, + name: osmUser?.name || '', + } + }), } } diff --git a/src/pages/api/organizations/[orgId]/staff.js b/src/pages/api/organizations/[orgId]/staff.js index 81240555..1df497b4 100644 --- a/src/pages/api/organizations/[orgId]/staff.js +++ b/src/pages/api/organizations/[orgId]/staff.js @@ -37,16 +37,15 @@ handler.get( canViewOrgMembers, validate({ query: Yup.object({ - orgId: Yup.number().required().positive().integer(), + orgId: Yup.number().required().positive().integer().required(), page: Yup.number().min(0).integer(), }).required(), }), async function (req, res) { const { orgId, page } = req.query - const staff = await Organization.getOrgStaff({ + const staff = await Organization.getOrgStaff(orgId, { page, - organizationId: orgId, }) return res.send(staff) From ac1eb337f4f13a4d0d61d9e27c1d8876b87c4546 Mon Sep 17 00:00:00 2001 From: Vitor George Date: Wed, 21 Dec 2022 09:03:06 +0000 Subject: [PATCH 07/24] Fix seed data --- cypress.config.js | 2 ++ src/models/team.js | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/cypress.config.js b/cypress.config.js index 042804c9..00abd2db 100644 --- a/cypress.config.js +++ b/cypress.config.js @@ -18,6 +18,8 @@ module.exports = defineConfig({ 'db:reset': async () => { await db.raw('TRUNCATE TABLE team RESTART IDENTITY CASCADE') await db.raw('TRUNCATE TABLE organization RESTART IDENTITY CASCADE') + await db.raw('TRUNCATE TABLE users RESTART IDENTITY CASCADE') + await db.raw('TRUNCATE TABLE osm_users RESTART IDENTITY CASCADE') return null }, 'db:seed': async () => { diff --git a/src/models/team.js b/src/models/team.js index 5a4b3558..2fcae33e 100644 --- a/src/models/team.js +++ b/src/models/team.js @@ -6,6 +6,7 @@ const xml2js = require('xml2js') const { unpack } = require('../../app/lib/utils') const { prop, isEmpty, difference, concat, assoc } = require('ramda') const request = require('request-promise-native') +const { addZeroPadding } = require('../lib/utils') const { serverRuntimeConfig } = require('../../next.config') const { DEFAULT_PAGE_SIZE } = serverRuntimeConfig @@ -79,7 +80,7 @@ async function resolveMemberNames(ids) { if (process.env.TESTING === 'true') { usersFromOSM = notFound.map((id) => ({ id, - name: `User ${id}`, + name: `User ${addZeroPadding(id, 3)}`, // use zero-padded number for consistent table sorting during tests image: `https://via.placeholder.com/150`, })) } else { From 9cb5615246848c32ef9f0867868d98f3af703dfb Mon Sep 17 00:00:00 2001 From: Vitor George Date: Wed, 21 Dec 2022 09:05:51 +0000 Subject: [PATCH 08/24] Create a folder for organizations-related Cypress specs --- .../{organizations.cy.js => organizations/pagination.cy.js} | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) rename cypress/e2e/{organizations.cy.js => organizations/pagination.cy.js} (98%) diff --git a/cypress/e2e/organizations.cy.js b/cypress/e2e/organizations/pagination.cy.js similarity index 98% rename from cypress/e2e/organizations.cy.js rename to cypress/e2e/organizations/pagination.cy.js index 729ddc7c..a9d2bd1f 100644 --- a/cypress/e2e/organizations.cy.js +++ b/cypress/e2e/organizations/pagination.cy.js @@ -1,4 +1,7 @@ -const { generateSequenceArray, addZeroPadding } = require('../../src/lib/utils') +const { + generateSequenceArray, + addZeroPadding, +} = require('../../../src/lib/utils') const orgStaff = generateSequenceArray(7, 1).map((i) => ({ id: i, From bfe7a0445d8b7153c9d64357d40cbf1a23e369a2 Mon Sep 17 00:00:00 2001 From: Vitor George Date: Wed, 21 Dec 2022 11:39:16 +0000 Subject: [PATCH 09/24] Add permissions spec --- app/manage/organizations.js | 12 ++- cypress.config.js | 25 +++-- cypress/e2e/organizations/pagination.cy.js | 2 +- cypress/e2e/organizations/permissions.cy.js | 113 ++++++++++++++++++++ src/components/tables/table.js | 8 +- src/pages/organizations/[id]/index.js | 86 ++++++++++----- 6 files changed, 208 insertions(+), 38 deletions(-) create mode 100644 cypress/e2e/organizations/permissions.cy.js diff --git a/app/manage/organizations.js b/app/manage/organizations.js index e7a00c2c..3a6081a9 100644 --- a/app/manage/organizations.js +++ b/app/manage/organizations.js @@ -45,11 +45,19 @@ async function getOrg(req, reply) { throw Boom.badRequest('organization id is required') } - let [data, isMemberOfOrg] = await Promise.all([ + let [data, isMember, isManager, isOwner] = await Promise.all([ organization.get(id), organization.isMember(id, user_id), + organization.isManager(id, user_id), + organization.isOwner(id, user_id), ]) - reply.send({ ...data, isMemberOfOrg }) + + // User needs to be member or staff to access a private org + if (data?.privacy === 'private' && !isMember && !isManager && !isOwner) { + throw Boom.unauthorized() + } else { + reply.send({ ...data, isMember, isManager, isOwner }) + } } /** diff --git a/cypress.config.js b/cypress.config.js index 00abd2db..d8f60f42 100644 --- a/cypress.config.js +++ b/cypress.config.js @@ -64,23 +64,30 @@ module.exports = defineConfig({ 'db:seed:team-invitations': async (teamInvitations) => { return Promise.all(teamInvitations.map(TeamInvitation.create)) }, - 'db:seed:organizations': async (orgs) => { - return Promise.all( - orgs.map((org) => - Organization.create(pick(['name'], org), org.ownerId) + 'db:seed:add-organizations': async (orgs) => { + for (let i = 0; i < orgs.length; i++) { + const org = orgs[i] + await Organization.create( + pick(['name', 'privacy'], org), + org.ownerId ) - ) + } + return null }, 'db:seed:add-organization-teams': async ({ orgId, teams, managerId, }) => { - return Promise.all( - teams.map((team) => - Organization.createOrgTeam(orgId, pick(['name'], team), managerId) + for (let i = 0; i < teams.length; i++) { + const team = teams[i] + await Organization.createOrgTeam( + orgId, + pick(['name', 'privacy'], team), + managerId ) - ) + } + return null }, 'db:seed:add-organization-managers': async ({ orgId, managerIds }) => { for (let i = 0; i < managerIds.length; i++) { diff --git a/cypress/e2e/organizations/pagination.cy.js b/cypress/e2e/organizations/pagination.cy.js index a9d2bd1f..b7046375 100644 --- a/cypress/e2e/organizations/pagination.cy.js +++ b/cypress/e2e/organizations/pagination.cy.js @@ -48,7 +48,7 @@ const orgTeam3Members = generateSequenceArray(15, 300).map((i) => ({ describe('Organization page', () => { before(() => { cy.task('db:reset') - cy.task('db:seed:organizations', [org1]) + cy.task('db:seed:add-organizations', [org1]) }) it('Display message when organization has no teams', () => { diff --git a/cypress/e2e/organizations/permissions.cy.js b/cypress/e2e/organizations/permissions.cy.js new file mode 100644 index 00000000..73b0c911 --- /dev/null +++ b/cypress/e2e/organizations/permissions.cy.js @@ -0,0 +1,113 @@ +const { + generateSequenceArray, + addZeroPadding, +} = require('../../../src/lib/utils') + +const orgStaff = generateSequenceArray(5, 1).map((i) => ({ + id: i, + name: `User ${addZeroPadding(i, 3)}`, +})) + +const [ownerUser, managerUser, orgTeamMember1] = orgStaff + +// ORGANIZATION SEED +const privateOrg = { + id: 1, + name: 'Private Org', + privacy: 'private', + ownerId: ownerUser.id, +} + +const publicOrg = { + id: 2, + name: 'Public Org', + ownerId: ownerUser.id, +} + +// ORGANIZATION TEAMS SEED +const privateOrgPrivateTeam = { + id: 1, + name: 'Private Org Private Team', + privacy: 'private', +} + +const privateOrgPublicTeam1 = { + id: 2, + name: 'Private Org Public Team 1', +} + +const privateOrgPublicTeam2 = { + id: 3, + name: 'Private Org Public Team 2', +} + +describe('Organizations page: Permissions', () => { + before(() => { + cy.task('db:reset') + }) + + it('Org is private', () => { + cy.task('db:seed:add-organizations', [privateOrg, publicOrg]) + + // Unauthenticated user cannot access + cy.visit('/organizations/1') + cy.get('body').should('contain', 'Sign in with OSM Teams') + + // Start owner permissions checks + cy.login(ownerUser) + cy.visit('/organizations/1') + + // Org is loaded and all tables are present + cy.get('body').should('contain', 'Private Org') + cy.get('[data-cy=org-staff-table]').should('exist') + cy.get('[data-cy=org-members-table]').should('exist') + cy.get('body').should('contain', 'Badges') + + // Manager permissions + cy.task('db:seed:add-organization-managers', { + orgId: privateOrg.id, + managerIds: [managerUser.id], + }) + cy.login(managerUser) + cy.visit('/organizations/1') + + // Org is loaded and badges table is not present + cy.get('body').should('contain', 'Private Org') + cy.get('[data-cy=org-staff-table]').should('exist') + cy.get('[data-cy=org-members-table]').should('exist') + cy.get('[data-cy=badges-table]').should('not.exist') + + // Create org teams + cy.task('db:seed:add-organization-teams', { + orgId: privateOrg.id, + teams: [ + privateOrgPrivateTeam, + privateOrgPublicTeam1, + privateOrgPublicTeam2, + ], + managerId: managerUser.id, + }) + + // Org team member can view its own team and the public ones + cy.task('db:seed:add-members-to-team', { + teamId: privateOrgPrivateTeam.id, + members: [orgTeamMember1], + }) + cy.login(orgTeamMember1) + cy.visit('/organizations/1') + + // Org is loaded + cy.get('body').should('contain', 'Private Org') + cy.get('[data-cy=org-staff-table]').should('not.exist') + cy.get('[data-cy=org-members-table]').should('not.exist') + cy.get('[data-cy=badges-table]').should('not.exist') + + // Can see its own team and public ones + cy.get('[data-cy=org-teams-table]').within(() => { + cy.get('[data-cy=not-empty-table]').should('exist') + cy.should('contain', privateOrgPrivateTeam.name) + cy.should('contain', privateOrgPublicTeam1.name) + cy.should('contain', privateOrgPublicTeam2.name) + }) + }) +}) diff --git a/src/components/tables/table.js b/src/components/tables/table.js index 6d8a7cbb..25a81d01 100644 --- a/src/components/tables/table.js +++ b/src/components/tables/table.js @@ -57,9 +57,13 @@ function TableBody({ emptyPlaceHolder, showRowNumbers, }) { + const isEmpty = !rows || rows.length === 0 return ( - - {!rows || rows.length === 0 ? ( + + {isEmpty ? (
{emptyPlaceHolder || 'No data available.'} diff --git a/src/pages/organizations/[id]/index.js b/src/pages/organizations/[id]/index.js index ad41cc92..8ab88a9b 100644 --- a/src/pages/organizations/[id]/index.js +++ b/src/pages/organizations/[id]/index.js @@ -56,6 +56,9 @@ class Organization extends Component { constructor(props) { super(props) this.state = { + org: { + status: 'idle', + }, profileInfo: [], profileUserId: '', teams: [], @@ -75,9 +78,7 @@ class Organization extends Component { async componentDidMount() { this.setState({ session: await getSession() }) await this.getOrg() - // await this.getOrgStaff() await this.getBadges() - // return this.getMembers(0) } async openProfileModal(user) { @@ -116,17 +117,26 @@ class Organization extends Component { async getOrg() { const { id } = this.props + this.setState({ + org: { + status: 'loading', + }, + }) + try { let org = await getOrg(id) this.setState({ - org, + org: { + data: org, + status: 'success', + }, }) } catch (e) { - console.error(e) this.setState({ - error: e, - org: null, - loading: false, + org: { + error: e, + status: 'error', + }, }) } } @@ -172,6 +182,7 @@ class Organization extends Component { {this.state.badges && ( { return { ...row, @@ -192,7 +203,28 @@ class Organization extends Component { render() { const { org, managers, owners, error } = this.state - if (!org) return null + + // Handle org loading errors + if (org.status === 'error') { + if (org.error.status === 401 || org.error.status === 403) { + return ( +
+

Unauthorized

+
+ ) + } else if (org.error.status === 404) { + return ( +
+

Org not found

+
+ ) + } + } + + // Do not render page until org is fetched + if (org.status !== 'success') { + return null + } const userId = parseInt(this.state.session.user_id) const ownerIds = map(parseInt, map(prop('id'), owners)) @@ -200,8 +232,8 @@ class Organization extends Component { const isUserOwner = contains(userId, ownerIds) const disabledLabel = !this.state.loading ? 'primary' : 'disabled' - const isOrgPublic = org.privacy === 'public' - const isMemberOfOrg = org.isMemberOfOrg + const { isManager, isOwner } = org.data + const isStaff = isManager || isOwner if (error) { if (error.status === 401 || error.status === 403) { @@ -235,7 +267,7 @@ class Organization extends Component { profileActions.push({ name: 'Remove owner', onClick: async () => { - await removeOwner(org.id, profileId) + await removeOwner(org.data.id, profileId) this.getOrg() }, }) @@ -245,14 +277,14 @@ class Organization extends Component { profileActions.push({ name: 'Promote to owner', onClick: async () => { - await addOwner(org.id, profileId) + await addOwner(org.data.id, profileId) this.getOrg() }, }) profileActions.push({ name: 'Remove manager', onClick: async () => { - await removeManager(org.id, profileId) + await removeManager(org.data.id, profileId) this.getOrg() }, }) @@ -263,7 +295,10 @@ class Organization extends Component { name: 'Assign a Badge', onClick: () => Router.push( - join(URL, `/organizations/${org.id}/badges/assign/${profileId}`) + join( + URL, + `/organizations/${org.data.id}/badges/assign/${profileId}` + ) ), }) } @@ -271,14 +306,17 @@ class Organization extends Component { return (
-

{org.name}

+

{org.data.name}

Org Details {isUserOwner ? ( - ) : ( @@ -287,7 +325,7 @@ class Organization extends Component {
Bio:
-
{org.description}
+
{org.data.description}
@@ -295,10 +333,10 @@ class Organization extends Component {
Teams
- + - {isOrgPublic || isMemberOfOrg ? ( + {isStaff ? (
@@ -307,7 +345,7 @@ class Organization extends Component { {isUserOwner && ( { - await addManager(org.id, osmId) + await addManager(org.data.id, osmId) return this.getOrg() }} /> @@ -315,12 +353,12 @@ class Organization extends Component {
- + ) : (
)} - {isOrgPublic || isMemberOfOrg ? ( + {isStaff ? (
@@ -349,12 +387,12 @@ class Organization extends Component {
- +
) : (
)} - {this.renderBadges()} + {isStaff && this.renderBadges()} Date: Wed, 21 Dec 2022 11:40:43 +0000 Subject: [PATCH 10/24] Fix staff spec --- tests/api/organization-api.test.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/api/organization-api.test.js b/tests/api/organization-api.test.js index bb600ea9..99317607 100644 --- a/tests/api/organization-api.test.js +++ b/tests/api/organization-api.test.js @@ -58,10 +58,13 @@ test('get organization staff', async (t) => { .send({ name: 'get organization staff' }) .expect(200) - const org = await user1Agent.get(`/api/organizations/${res.body.id}/staff`) + const { + body: { + pagination: { total }, + }, + } = await user1Agent.get(`/api/organizations/${res.body.id}/staff`) - t.is(org.body.owners.length, 1) - t.is(org.body.managers.length, 1) + t.is(total, 1) }) /** From 9a0509d9f5463ea89dcc2e23e156235e147d2394 Mon Sep 17 00:00:00 2001 From: Vitor George Date: Wed, 21 Dec 2022 14:44:39 +0000 Subject: [PATCH 11/24] Remove deprecated pagination buttons --- src/pages/organizations/[id]/index.js | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/src/pages/organizations/[id]/index.js b/src/pages/organizations/[id]/index.js index 8ab88a9b..4291d378 100644 --- a/src/pages/organizations/[id]/index.js +++ b/src/pages/organizations/[id]/index.js @@ -363,31 +363,9 @@ class Organization extends Component {
Organization Members -
- - {this.state.page > 0 ? ( - - ) : ( - '' - )} - - -
+
-
) : (
From 14a467a4a83d83979d2620cb34ce54c4b95c135b Mon Sep 17 00:00:00 2001 From: Vitor George Date: Wed, 21 Dec 2022 14:49:11 +0000 Subject: [PATCH 12/24] Non-member cannot view private organization --- cypress/e2e/organizations/permissions.cy.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/cypress/e2e/organizations/permissions.cy.js b/cypress/e2e/organizations/permissions.cy.js index 73b0c911..2aa0d11e 100644 --- a/cypress/e2e/organizations/permissions.cy.js +++ b/cypress/e2e/organizations/permissions.cy.js @@ -3,12 +3,12 @@ const { addZeroPadding, } = require('../../../src/lib/utils') -const orgStaff = generateSequenceArray(5, 1).map((i) => ({ +const users = generateSequenceArray(4, 1).map((i) => ({ id: i, name: `User ${addZeroPadding(i, 3)}`, })) -const [ownerUser, managerUser, orgTeamMember1] = orgStaff +const [ownerUser, managerUser, orgTeamMember1, nonMember] = users // ORGANIZATION SEED const privateOrg = { @@ -53,6 +53,11 @@ describe('Organizations page: Permissions', () => { cy.visit('/organizations/1') cy.get('body').should('contain', 'Sign in with OSM Teams') + // Non-member cannot access + cy.login(nonMember) + cy.visit('/organizations/1') + cy.get('body').should('contain', 'Unauthorized') + // Start owner permissions checks cy.login(ownerUser) cy.visit('/organizations/1') From 8fc5d2e67cfda9de33018568c1fbf65ef0eba79c Mon Sep 17 00:00:00 2001 From: Vitor George Date: Wed, 21 Dec 2022 16:14:48 +0000 Subject: [PATCH 13/24] Remove deprecated dockerfile --- Dockerfile | 18 ------------------ 1 file changed, 18 deletions(-) delete mode 100644 Dockerfile diff --git a/Dockerfile b/Dockerfile deleted file mode 100644 index 47bb0bfe..00000000 --- a/Dockerfile +++ /dev/null @@ -1,18 +0,0 @@ -FROM node:14 - -# Create app directory -WORKDIR /usr/src/app - -# Install app dependencies -COPY package.json ./ - -RUN npm install -g nodemon -RUN npm install - -# Bundle app source -COPY . . - -RUN npm run build - -EXPOSE 3000 -CMD [ "npm", "start"] From f703b084b89803509b44e9be3e8ee27f0411d2b8 Mon Sep 17 00:00:00 2001 From: Vitor George Date: Wed, 21 Dec 2022 16:15:40 +0000 Subject: [PATCH 14/24] Open Cypress e2e mode by default when running 'yarn e2e:dev' --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index ac9988c2..ffed9a4b 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,7 @@ "docs:update-version": "node -p \"JSON.stringify({'apiFolder': 'src','schemaFolders': ['src'],'definition': {'openapi': '3.0.0','info': {'title': 'OSM Teams API Docs','version': require('./package.json').version}}}, null, 2)\" > next-swagger-doc.json", "docs:generate": "yarn next-swagger-doc-cli next-swagger-doc.json", "docs:validate": "yarn docs:update-version && yarn docs:generate && swagger-cli validate public/swagger.json", - "cy:open": "cypress open", + "cy:open": "cypress open --e2e", "cy:run": "cypress run", "e2e:dev": "NODE_ENV=test start-server-and-test dev http://127.0.0.1:3000/api cy:open", "e2e": "NODE_ENV=test start-server-and-test dev http://127.0.0.1:3000/api cy:run", From b4a9e0368f4c75fe7a4f4f37bedee439d33e5382 Mon Sep 17 00:00:00 2001 From: Vitor George Date: Wed, 21 Dec 2022 16:32:57 +0000 Subject: [PATCH 15/24] Add canViewOrgTeams middleware, improve test coverage --- cypress.config.js | 2 +- cypress/e2e/organizations/permissions.cy.js | 93 +++++++++++++++++++- src/middlewares/can/view-org-teams.js | 30 +++++++ src/models/team.js | 13 ++- src/pages/api/organizations/[orgId]/teams.js | 15 +++- src/pages/organizations/[id]/index.js | 8 +- 6 files changed, 149 insertions(+), 12 deletions(-) create mode 100644 src/middlewares/can/view-org-teams.js diff --git a/cypress.config.js b/cypress.config.js index d8f60f42..fd6b30fa 100644 --- a/cypress.config.js +++ b/cypress.config.js @@ -83,7 +83,7 @@ module.exports = defineConfig({ const team = teams[i] await Organization.createOrgTeam( orgId, - pick(['name', 'privacy'], team), + pick(['id', 'name', 'privacy'], team), managerId ) } diff --git a/cypress/e2e/organizations/permissions.cy.js b/cypress/e2e/organizations/permissions.cy.js index 2aa0d11e..e717c89b 100644 --- a/cypress/e2e/organizations/permissions.cy.js +++ b/cypress/e2e/organizations/permissions.cy.js @@ -41,14 +41,30 @@ const privateOrgPublicTeam2 = { name: 'Private Org Public Team 2', } +// ORGANIZATION TEAMS SEED +const publicOrgPrivateTeam = { + id: 4, + name: 'Public Org Private Team', + privacy: 'private', +} + +const publicOrgPublicTeam1 = { + id: 5, + name: 'Public Org Public Team 1', +} + +const publicOrgPublicTeam2 = { + id: 6, + name: 'Public Org Public Team 2', +} + describe('Organizations page: Permissions', () => { before(() => { cy.task('db:reset') + cy.task('db:seed:add-organizations', [privateOrg, publicOrg]) }) it('Org is private', () => { - cy.task('db:seed:add-organizations', [privateOrg, publicOrg]) - // Unauthenticated user cannot access cy.visit('/organizations/1') cy.get('body').should('contain', 'Sign in with OSM Teams') @@ -115,4 +131,77 @@ describe('Organizations page: Permissions', () => { cy.should('contain', privateOrgPublicTeam2.name) }) }) + + it('Org is public', () => { + // Create org teams + cy.task('db:seed:add-organization-teams', { + orgId: publicOrg.id, + teams: [publicOrgPrivateTeam, publicOrgPublicTeam1, publicOrgPublicTeam2], + managerId: managerUser.id, + }) + + // Unauthenticated can view org + cy.visit('/organizations/2') + cy.get('body').should('contain', 'Sign in with OSM Teams') + + // Non-member can access, but cannot view private teams + cy.login(nonMember) + cy.visit('/organizations/2') + cy.get('body').should('contain', 'Public Org') + cy.get('[data-cy=org-staff-table]').should('not.exist') + cy.get('[data-cy=org-members-table]').should('not.exist') + cy.get('body').should('not.contain', 'Badges') + cy.get('[data-cy=org-teams-table]').within(() => { + cy.get('[data-cy=not-empty-table]').should('exist') + cy.should('not.contain', publicOrgPrivateTeam.name) + cy.should('contain', publicOrgPublicTeam1.name) + cy.should('contain', publicOrgPublicTeam2.name) + }) + + // Start owner permissions checks + cy.login(ownerUser) + cy.visit('/organizations/2') + + // Org is loaded and all tables are present + cy.get('body').should('contain', 'Public Org') + cy.get('[data-cy=org-staff-table]').should('exist') + cy.get('[data-cy=org-members-table]').should('exist') + cy.get('body').should('contain', 'Badges') + + // Manager permissions + cy.task('db:seed:add-organization-managers', { + orgId: publicOrg.id, + managerIds: [managerUser.id], + }) + cy.login(managerUser) + cy.visit('/organizations/2') + + // Org is loaded and badges table is not present + cy.get('body').should('contain', 'Public Org') + cy.get('[data-cy=org-staff-table]').should('exist') + cy.get('[data-cy=org-members-table]').should('exist') + cy.get('[data-cy=badges-table]').should('not.exist') + + // Org team member can view its own team and the public ones + cy.task('db:seed:add-members-to-team', { + teamId: publicOrgPrivateTeam.id, + members: [orgTeamMember1], + }) + cy.login(orgTeamMember1) + cy.visit('/organizations/2') + + // Org is loaded + cy.get('body').should('contain', 'Public Org') + cy.get('[data-cy=org-staff-table]').should('not.exist') + cy.get('[data-cy=org-members-table]').should('not.exist') + cy.get('[data-cy=badges-table]').should('not.exist') + + // Can see its own team and public ones + cy.get('[data-cy=org-teams-table]').within(() => { + cy.get('[data-cy=not-empty-table]').should('exist') + cy.should('contain', publicOrgPrivateTeam.name) + cy.should('contain', publicOrgPublicTeam1.name) + cy.should('contain', publicOrgPublicTeam2.name) + }) + }) }) diff --git a/src/middlewares/can/view-org-teams.js b/src/middlewares/can/view-org-teams.js new file mode 100644 index 00000000..78932814 --- /dev/null +++ b/src/middlewares/can/view-org-teams.js @@ -0,0 +1,30 @@ +import Boom from '@hapi/boom' +import Organization from '../../models/organization' + +/** + * Permission middleware to check if user can view organization teams. + * Add permission flags to request object (isMember, isManager, isOwner) + * @param {object} req The request + * @param {object} res The response + * @param {function} next The next middleware + * @returns The results of the next middleware + */ +export default async function canViewOrgTeams(req, res, next) { + const { orgId } = req.query + const { user_id: userId } = req.session + + let [org, isMember, isManager, isOwner] = await Promise.all([ + Organization.get(orgId), + Organization.isMember(orgId, userId), + Organization.isManager(orgId, userId), + Organization.isOwner(orgId, userId), + ]) + + if (org?.privacy === 'public' || isMember || isManager || isOwner) { + // Add org and permission flags to request + req.org = { ...org, isMember, isManager, isOwner } + return next() + } else { + throw Boom.unauthorized() + } +} diff --git a/src/models/team.js b/src/models/team.js index 2fcae33e..81542cb5 100644 --- a/src/models/team.js +++ b/src/models/team.js @@ -195,11 +195,22 @@ async function count({ organizationId }) { * @return {Promise[Array]} **/ async function list(options = {}) { - const { bbox, osmId, organizationId, page, disablePagination } = options + const { + bbox, + osmId, + organizationId, + page, + disablePagination, + includePrivate, + } = options const st = knexPostgis(db) let query = db('team').select(...teamAttributes, st.asGeoJSON('location')) + if (!includePrivate) { + query.where('privacy', 'public') + } + if (osmId) { query = query.whereIn('id', function () { this.select('team_id').from('member').where('osm_id', osmId) diff --git a/src/pages/api/organizations/[orgId]/teams.js b/src/pages/api/organizations/[orgId]/teams.js index efa7a482..e04696ae 100644 --- a/src/pages/api/organizations/[orgId]/teams.js +++ b/src/pages/api/organizations/[orgId]/teams.js @@ -3,8 +3,8 @@ import { validate } from '../../../../middlewares/validation' import Organization from '../../../../models/organization' import Team from '../../../../models/team' import * as Yup from 'yup' -import canViewOrgMembers from '../../../../middlewares/can/view-org-members' import canCreateOrgTeam from '../../../../middlewares/can/create-org-team' +import canViewOrgTeams from '../../../../middlewares/can/view-org-teams' const handler = createBaseHandler() @@ -91,7 +91,7 @@ handler.post( * $ref: '#/components/schemas/ArrayOfTeams' */ handler.get( - canViewOrgMembers, + canViewOrgTeams, validate({ query: Yup.object({ orgId: Yup.number().required().positive().integer(), @@ -100,7 +100,16 @@ handler.get( }), async function (req, res) { const { orgId, page } = req.query - return res.send(await Team.list({ organizationId: orgId, page })) + const { + org: { isMember, isOwner, isManager }, + } = req + return res.send( + await Team.list({ + organizationId: orgId, + page, + includePrivate: isMember || isManager || isOwner, + }) + ) } ) diff --git a/src/pages/organizations/[id]/index.js b/src/pages/organizations/[id]/index.js index 4291d378..8006cdbd 100644 --- a/src/pages/organizations/[id]/index.js +++ b/src/pages/organizations/[id]/index.js @@ -229,8 +229,6 @@ class Organization extends Component { const userId = parseInt(this.state.session.user_id) const ownerIds = map(parseInt, map(prop('id'), owners)) const managerIds = map(parseInt, map(prop('id'), managers)) - const isUserOwner = contains(userId, ownerIds) - const disabledLabel = !this.state.loading ? 'primary' : 'disabled' const { isManager, isOwner } = org.data const isStaff = isManager || isOwner @@ -259,7 +257,7 @@ class Organization extends Component { let profileActions = [] - if (this.state.modalIsOpen && isUserOwner) { + if (this.state.modalIsOpen && isOwner) { const profileId = parseInt(this.state.profileMeta.id) const isProfileManager = contains(profileId, managerIds) const isProfileOwner = contains(profileId, ownerIds) @@ -312,7 +310,7 @@ class Organization extends Component {
Org Details - {isUserOwner ? ( + {isOwner ? (
- + this.openProfileModal(row)} + />
) : (
@@ -362,7 +386,11 @@ class Organization extends Component {
Organization Members
- + this.openProfileModal(row)} + />
) : ( From 542ca63d6da705aa9ec9d28224ebf258c2be7017 Mon Sep 17 00:00:00 2001 From: Lane Goodman Date: Thu, 22 Dec 2022 12:18:43 -0800 Subject: [PATCH 23/24] Fix webkit-specific overflow property --- src/components/layout.js | 2 +- src/styles/global.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/layout.js b/src/components/layout.js index 015328ce..99f30eae 100644 --- a/src/components/layout.js +++ b/src/components/layout.js @@ -44,7 +44,7 @@ export const globalStyles = css.global` 'main' 'footer'; height: 100vh; - overflow: overlay; + overflow: auto; } @media screen and (min-width: ${theme.mediaRanges.small}) { diff --git a/src/styles/global.js b/src/styles/global.js index 6dd491ba..f4bd523c 100644 --- a/src/styles/global.js +++ b/src/styles/global.js @@ -43,7 +43,7 @@ export default css` 'main' 'footer'; height: 100vh; - overflow: overlay; + overflow: auto; } .inner { From bfe62c612b911dc7b99522e58633161c429c00ef Mon Sep 17 00:00:00 2001 From: Marc Farra Date: Fri, 23 Dec 2022 10:44:26 +0200 Subject: [PATCH 24/24] quick fix bug in badge page --- src/pages/organizations/[id]/badges/[badgeId]/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/organizations/[id]/badges/[badgeId]/index.js b/src/pages/organizations/[id]/badges/[badgeId]/index.js index 91786f56..49f7f28f 100644 --- a/src/pages/organizations/[id]/badges/[badgeId]/index.js +++ b/src/pages/organizations/[id]/badges/[badgeId]/index.js @@ -39,7 +39,7 @@ export default class EditBadge extends Component { static async getInitialProps({ query }) { if (query) { return { - orgId: query.badgeId, + orgId: query.id, badgeId: query.badgeId, } }