Skip to content

Commit e58e48c

Browse files
authored
feat(api): schools config (#53)
1 parent c1c20ae commit e58e48c

34 files changed

+859
-53
lines changed
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
# ADR: School Configurations
2+
3+
We need a method to store and manage school-specific configurations, such as student roles and default student role settings. The configuration should be easily retrievable and updatable through API endpoints while maintaining simplicity in the implementation given our current requirements.
4+
5+
## Decision
6+
We will implement school configurations using the following approach:
7+
8+
1. **Configuration Structure**:
9+
- Configurations will be stored as JSON objects
10+
- Example format: `{ studentRoles: [1,2,3], defaultStudentRole: 1 }`
11+
- This allows flexible key-value pairs to accommodate current needs (student roles) and potential future configuration parameters
12+
13+
2. **API Endpoints**:
14+
- `GET /edu/schools/:guid/config`: Retrieve the complete configuration for a specific school
15+
- `PATCH /edu/schools/:guid/config`: Update specific configuration values for a school
16+
- Using RESTful conventions with school GUID as the identifier
17+
18+
3. **Storage**:
19+
- Configurations will be stored in the existing `Schools` table as a JSON column
20+
- This leverages existing infrastructure and maintains data locality with school records
21+
22+
## Consequences
23+
- **Positive**
24+
- Simple implementation using existing database infrastructure
25+
- Single table access for school data and configuration
26+
- Flexible JSON structure allows for future configuration additions
27+
- Standard RESTful API endpoints for easy integration
28+
- Atomic updates possible through PATCH operations
29+
- Payload of `PATCH /edu/schools/:guid/config` can be validated by NestJS framework
30+
31+
- **Negative**
32+
- Potential size limitations with JSON column in database
33+
- Less granular control compared to separate configuration tables
34+
- Schema validation must be handled in application layer
35+
36+
## Rejected Alternatives
37+
38+
1. **Separate SchoolConfig Table**
39+
- Description: Create a dedicated table for configurations with foreign key to Schools
40+
- Rejected because:
41+
- Adds unnecessary complexity for current simple configuration needs
42+
- Requires additional joins for data retrieval
43+
- More schema maintenance overhead
44+
45+
2. **External Storage (Redis)**
46+
- Description: Store configurations in a separate key-value store like Redis
47+
- Rejected because:
48+
- Introduces additional infrastructure component to maintain
49+
- Increases operational complexity
50+
- Overkill for current configuration volume and access patterns
51+
- Adds deployment and monitoring overhead

modules/libs/entities/school.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,17 @@
1-
import { Entity, Column, PrimaryGeneratedColumn, ManyToOne, JoinColumn } from 'typeorm';
1+
import { Entity, Column, PrimaryGeneratedColumn } from 'typeorm';
2+
3+
export type SchoolConfig = {
4+
/**
5+
* The default role for students in this school.
6+
* This role will be assigned to students when they are joined.
7+
*/
8+
defaultStudentRoleId: string;
9+
10+
/**
11+
* List of student roles that are available in this school.
12+
*/
13+
studentRoleIds: string[];
14+
};
215

316
@Entity({ name: 'schools' })
417
export class School {
@@ -7,4 +20,7 @@ export class School {
720

821
@Column({ nullable: false })
922
name: string;
23+
24+
@Column({ nullable: false, type: 'json', default: {} })
25+
config: SchoolConfig;
1026
}

modules/libs/protocol/routes.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,18 @@ export const Routes = (baseUrl: string = '') => ({
3030
all: () => `${baseUrl}/edu/users/${userId}/roles`,
3131
create: () => `${baseUrl}/edu/users/${userId}/roles`,
3232
delete: (roleId: string) => `${baseUrl}/edu/users/${userId}/roles/${roleId}`,
33+
},
34+
schools: {
35+
all: () => `${baseUrl}/edu/users/${userId}/schools`,
36+
create: () => `${baseUrl}/edu/users/${userId}/schools`,
37+
delete: (roleId: string) => `${baseUrl}/edu/users/${userId}/schools/${roleId}`,
3338
}
3439
}),
3540
schools: {
41+
configs: {
42+
getAll: (schoolId: string) => `${baseUrl}/edu/schools/${schoolId}/configs`,
43+
update: (schoolId: string) => `${baseUrl}/edu/schools/${schoolId}/configs`,
44+
},
3645
find: () => `${baseUrl}/edu/schools`,
3746
get: (id: string) => `${baseUrl}/edu/schools/${id}`,
3847
create: () => `${baseUrl}/edu/schools`,

modules/services/api/src/edu/controllers/roles/roles.controller.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { Mapper } from '@automapper/core';
22
import { InjectMapper } from '@automapper/nestjs';
33
import {
4+
BadRequestException,
45
Body,
56
Controller,
67
ForbiddenException,
@@ -172,6 +173,10 @@ export class RolesController {
172173
throw new ForbiddenException('User does not have permission');
173174
}
174175

176+
if (role && role.permissions.includes('*')) {
177+
throw new BadRequestException('Cannot delete Owner role');
178+
}
179+
175180
// Delete role
176181
await this.rolesService.deleteOneBy({ id });
177182

modules/services/api/src/edu/controllers/roles/specs/context.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@ export type Context = {
88
one: {
99
school: School;
1010
roles: {
11-
admin: Role;
11+
owner: Role;
1212
readonly: Role;
1313
};
1414
tokens: {
15-
admin: string;
15+
owner: string;
1616
readOnly: string;
1717
oneAndTwoAdmin: string;
1818
};
@@ -67,10 +67,10 @@ export const createContext = async (
6767
/* Roles */
6868
/* -------------------------------------------------------------------------- */
6969

70-
const oneAdminRole = await rolesService.create({
71-
name: 'One :: Admin',
72-
description: 'Admin role for school one',
73-
permissions: ['roles:create', 'roles:read', 'roles:update', 'roles:delete'],
70+
const oneOwnerRole = await rolesService.create({
71+
name: 'One :: Owner',
72+
description: 'Owner role for school one',
73+
permissions: ['*'],
7474
schoolId: schoolOne.id,
7575
});
7676

@@ -92,8 +92,8 @@ export const createContext = async (
9292
/* Tokens */
9393
/* -------------------------------------------------------------------------- */
9494

95-
const oneTokenAdmin = await authService.generateTokens(faker.string.uuid(), [
96-
{ sid: schoolOne.id, p: oneAdminRole.permissions },
95+
const oneOwnerAdmin = await authService.generateTokens(faker.string.uuid(), [
96+
{ sid: schoolOne.id, p: oneOwnerRole.permissions },
9797
]);
9898

9999
const oneTokenReadonly = await authService.generateTokens(
@@ -113,7 +113,7 @@ export const createContext = async (
113113
);
114114

115115
const oneAndTwoAdmin = await authService.generateTokens(faker.string.uuid(), [
116-
{ sid: schoolOne.id, p: oneAdminRole.permissions },
116+
{ sid: schoolOne.id, p: oneOwnerRole.permissions },
117117
{ sid: schoolTwo.id, p: twoAdminRole.permissions },
118118
]);
119119

@@ -130,11 +130,11 @@ export const createContext = async (
130130
one: {
131131
school: schoolOne,
132132
roles: {
133-
admin: oneAdminRole,
133+
owner: oneOwnerRole,
134134
readonly: oneReadonlyRole,
135135
},
136136
tokens: {
137-
admin: oneTokenAdmin.accessToken,
137+
owner: oneOwnerAdmin.accessToken,
138138
readOnly: oneTokenReadonly.accessToken,
139139
oneAndTwoAdmin: oneAndTwoAdmin.accessToken,
140140
},

modules/services/api/src/edu/controllers/roles/specs/roles.controller.e2e.create.spec.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,37 @@ describe('/edu/roles', () => {
133133

134134
request(app.getHttpServer())
135135
.post(protocol.Routes().edu.roles.create())
136-
.set('Authorization', `Bearer ${ctx.one.tokens.admin}`)
136+
.set('Authorization', `Bearer ${ctx.one.tokens.owner}`)
137137
.send(payload)
138138
.expect(201)
139139
.expect((res) => {
140140
expect(res.body).toHaveProperty('id');
141141
expect(res.body.id).toBeDefined();
142142
});
143143
});
144+
145+
/* -------------------------------------------------------------------------- */
146+
/* Negative Cases */
147+
/* -------------------------------------------------------------------------- */
148+
149+
it(`POST /edu/roles returns 400 if permissions contain *`, async () => {
150+
const payload: protocol.CreateRoleRequest = {
151+
name: 'New Role',
152+
description: 'Role description',
153+
permissions: ['*'],
154+
schoolId: ctx.one.school.id,
155+
};
156+
157+
return request(app.getHttpServer())
158+
.post(protocol.Routes().edu.roles.create())
159+
.set('Authorization', `Bearer ${ctx.one.tokens.owner}`)
160+
.send(payload)
161+
.expect(400)
162+
.expect((res) => {
163+
expect(res.body).toHaveProperty('message');
164+
expect(res.body.message).toStrictEqual([
165+
'permissions cannot contain *',
166+
]);
167+
});
168+
});
144169
});

modules/services/api/src/edu/controllers/roles/specs/roles.controller.e2e.delete.spec.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ describe('/edu/roles', () => {
3030
it(`DELETE /edu/roles/:id 400 if id is invalid format`, async () => {
3131
return request(app.getHttpServer())
3232
.delete(Routes().edu.roles.delete('invalid-uuid'))
33-
.set('Authorization', `Bearer ${ctx.one.tokens.admin}`)
33+
.set('Authorization', `Bearer ${ctx.one.tokens.owner}`)
3434
.expect(400)
3535
.expect((res) => {
3636
expect(res.body).toHaveProperty('message');
@@ -44,7 +44,7 @@ describe('/edu/roles', () => {
4444

4545
it(`DELETE /edu/roles/:id returns 401 for unauthorized user`, async () => {
4646
return request(app.getHttpServer())
47-
.delete(Routes().edu.roles.delete(ctx.one.roles.admin.id))
47+
.delete(Routes().edu.roles.delete(ctx.one.roles.owner.id))
4848
.expect(401);
4949
});
5050

@@ -55,7 +55,7 @@ describe('/edu/roles', () => {
5555
it('DELETE /edu/roles/:id returns 403 for unauthorized user', async () => {
5656
return request(app.getHttpServer())
5757
.delete(Routes().edu.roles.delete(ctx.two.roles.admin.id))
58-
.set('Authorization', `Bearer ${ctx.one.tokens.admin}`)
58+
.set('Authorization', `Bearer ${ctx.one.tokens.owner}`)
5959
.expect(403)
6060
.expect((res) => {
6161
expect(res.body).toHaveProperty('message');
@@ -69,8 +69,8 @@ describe('/edu/roles', () => {
6969

7070
it(`DELETE /edu/roles/:id deletes an existing role`, async () => {
7171
return request(app.getHttpServer())
72-
.delete(Routes().edu.roles.delete(ctx.one.roles.admin.id))
73-
.set('Authorization', `Bearer ${ctx.one.tokens.admin}`)
72+
.delete(Routes().edu.roles.delete(ctx.two.roles.admin.id))
73+
.set('Authorization', `Bearer ${ctx.two.tokens.admin}`)
7474
.expect(200)
7575
.expect((res) => {
7676
expect(res.body).toHaveProperty('success');
@@ -82,11 +82,23 @@ describe('/edu/roles', () => {
8282
/* Negative Cases */
8383
/* -------------------------------------------------------------------------- */
8484

85+
it(`DELETE /edu/roles/:id deletes an existing role`, async () => {
86+
return request(app.getHttpServer())
87+
.delete(Routes().edu.roles.delete(ctx.one.roles.owner.id))
88+
.set('Authorization', `Bearer ${ctx.one.tokens.owner}`)
89+
.expect(400)
90+
.expect({
91+
message: 'Cannot delete Owner role',
92+
error: 'Bad Request',
93+
statusCode: 400,
94+
});
95+
});
96+
8597
it(`DELETE /edu/roles/:id returns 404 for non-existent role`, async () => {
8698
const nonExistentId = faker.string.uuid();
8799
return request(app.getHttpServer())
88100
.delete(Routes().edu.roles.delete(nonExistentId))
89-
.set('Authorization', `Bearer ${ctx.one.tokens.admin}`)
101+
.set('Authorization', `Bearer ${ctx.one.tokens.owner}`)
90102
.expect(404)
91103
.expect((res) => {
92104
expect(res.body).toHaveProperty('message');

modules/services/api/src/edu/controllers/roles/specs/roles.controller.e2e.get.spec.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,12 @@ describe('/edu/roles', () => {
4646
it(`GET /edu/roles returns only permitted roles`, async () => {
4747
return request(app.getHttpServer())
4848
.get(Routes().edu.roles.find())
49-
.set('Authorization', `Bearer ${ctx.one.tokens.admin}`)
49+
.set('Authorization', `Bearer ${ctx.one.tokens.owner}`)
5050
.expect(200)
5151
.expect({
5252
items: instanceToPlain(
5353
mapper.mapArray(
54-
[ctx.one.roles.admin, ctx.one.roles.readonly],
54+
[ctx.one.roles.owner, ctx.one.roles.readonly],
5555
entities.Role,
5656
dto.RoleSummary,
5757
),
@@ -67,7 +67,7 @@ describe('/edu/roles', () => {
6767
.expect({
6868
items: instanceToPlain(
6969
mapper.mapArray(
70-
[ctx.one.roles.admin, ctx.one.roles.readonly, ctx.two.roles.admin],
70+
[ctx.one.roles.owner, ctx.one.roles.readonly, ctx.two.roles.admin],
7171
entities.Role,
7272
dto.RoleSummary,
7373
),

modules/services/api/src/edu/controllers/roles/specs/roles.controller.e2e.update.spec.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ describe('/edu/roles', () => {
4141

4242
it(`PATCH /edu/roles/:id returns 401 for unauthorized user`, async () => {
4343
return request(app.getHttpServer())
44-
.patch(Routes().edu.roles.update(ctx.one.roles.admin.id))
44+
.patch(Routes().edu.roles.update(ctx.one.roles.owner.id))
4545
.expect(401);
4646
});
4747

@@ -53,7 +53,7 @@ describe('/edu/roles', () => {
5353
const updatedRole = { name: 'Updated Role' };
5454

5555
return request(app.getHttpServer())
56-
.patch(Routes().edu.roles.update(ctx.one.roles.admin.id))
56+
.patch(Routes().edu.roles.update(ctx.one.roles.owner.id))
5757
.set('Authorization', `Bearer ${ctx.one.tokens.readOnly}`)
5858
.send(updatedRole)
5959
.expect(403)
@@ -88,6 +88,10 @@ describe('/edu/roles', () => {
8888
payload: { description: faker.lorem.paragraphs(5) },
8989
errors: ['description must be shorter than or equal to 256 characters'],
9090
},
91+
{
92+
payload: { permissions: ['*'] },
93+
errors: ['permissions cannot contain *'],
94+
},
9195
// Permissions
9296
{
9397
payload: { permissions: ['invalid'] },
@@ -100,8 +104,8 @@ describe('/edu/roles', () => {
100104
`PATCH /edu/roles/:id 400 if payload is invalid`,
101105
async ({ payload, errors }) => {
102106
return request(app.getHttpServer())
103-
.patch(Routes().edu.roles.update(ctx.one.roles.admin.id))
104-
.set('Authorization', `Bearer ${ctx.one.tokens.admin}`)
107+
.patch(Routes().edu.roles.update(ctx.one.roles.owner.id))
108+
.set('Authorization', `Bearer ${ctx.one.tokens.owner}`)
105109
.send(payload)
106110
.expect(400)
107111
.expect((res) => {
@@ -121,8 +125,8 @@ describe('/edu/roles', () => {
121125
{ name: '#$#$#$%$#%^' },
122126
])(`PATCH /edu/roles/:id updates an existing role`, async (payload) => {
123127
return request(app.getHttpServer())
124-
.patch(Routes().edu.roles.update(ctx.one.roles.admin.id))
125-
.set('Authorization', `Bearer ${ctx.one.tokens.admin}`)
128+
.patch(Routes().edu.roles.update(ctx.one.roles.owner.id))
129+
.set('Authorization', `Bearer ${ctx.one.tokens.owner}`)
126130
.send(payload)
127131
.expect(200)
128132
.expect((res) => {

0 commit comments

Comments
 (0)