Skip to content

Commit 6b1bd10

Browse files
VeikkosuhonenVeikkosuhonen
committed
make rag use available to students & test refactoring
1 parent 428a5d8 commit 6b1bd10

File tree

10 files changed

+106
-82
lines changed

10 files changed

+106
-82
lines changed

‎e2e/chat.spec.ts‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect } from '@playwright/test'
2-
import { acceptDisclaimer } from './utils/test-helpers'
2+
import { acceptDisclaimer, closeSendPreference, sendChatMessage } from './utils/test-helpers'
33
import { teacherTest as test } from './fixtures'
44

55
test.describe('Chat v2 Conversation tests', () => {
@@ -12,12 +12,9 @@ test.describe('Chat v2 Conversation tests', () => {
1212
await page.getByTestId('model-selector').first().click()
1313
await page.getByRole('option', { name: 'mock' }).click()
1414

15-
const chatInput = page.getByTestId('chat-input').first()
16-
await chatInput.fill('testinen morjens')
17-
await chatInput.press('Shift+Enter')
15+
await sendChatMessage(page, 'testinen morjens')
1816

19-
// Close send preference configurator
20-
await page.locator('#send-preference-configurator-submit').click()
17+
await closeSendPreference(page)
2118

2219
await expect(page.getByTestId('user-message')).toContainText('testinen morjens')
2320
await expect(page.getByTestId('assistant-message')).toContainText('You are calling mock endpoint for streaming mock data')
@@ -27,12 +24,9 @@ test.describe('Chat v2 Conversation tests', () => {
2724
await page.getByTestId('model-selector').first().click()
2825
await page.getByRole('option', { name: 'mock' }).click()
2926

30-
const chatInput = page.getByTestId('chat-input').first()
31-
await chatInput.fill('tää tyhjennetään')
32-
await chatInput.press('Shift+Enter')
27+
await sendChatMessage(page, 'tää tyhjennetään')
3328

34-
// Close send preference configurator
35-
await page.locator('#send-preference-configurator-submit').click()
29+
await closeSendPreference(page)
3630

3731
await expect(page.getByTestId('user-message')).toContainText('tää tyhjennetään')
3832
await expect(page.getByTestId('assistant-message')).toContainText('OVER', { timeout: 6000 })

‎e2e/courseChatRag.spec.ts‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect } from '@playwright/test'
2-
import { acceptDisclaimer } from './utils/test-helpers'
2+
import { acceptDisclaimer, closeSendPreference, sendChatMessage } from './utils/test-helpers'
33
import { studentTest as test } from './fixtures'
44

55
test.describe('Course Chat v2', () => {
@@ -12,9 +12,8 @@ test.describe('Course Chat v2', () => {
1212
await page.getByTestId('model-selector').first().click()
1313
await page.getByRole('option', { name: 'mock' }).click()
1414

15-
const chatInput = page.getByTestId('chat-input').first()
16-
await chatInput.fill('testinen morjens')
17-
await chatInput.press('Shift+Enter')
15+
await sendChatMessage(page, 'testinen morjens')
16+
await closeSendPreference(page)
1817

1918
await expect(page.getByTestId('user-message')).toContainText('testinen morjens')
2019
await expect(page.getByTestId('assistant-message')).toContainText('You are calling mock endpoint for streaming mock data')
@@ -25,9 +24,8 @@ test.describe('Course Chat v2', () => {
2524
await page.locator('#rag-index-selector').first().click()
2625
await page.getByRole('menuitem', { name: ragName }).click()
2726

28-
const chatInput = page.getByTestId('chat-input').first()
29-
await chatInput.fill('rag')
30-
await chatInput.press('Shift+Enter')
27+
await sendChatMessage(page, 'rag')
28+
await closeSendPreference(page)
3129

3230
// Shows file search loading indicator
3331
await expect(page.getByTestId('tool-call-message')).toBeVisible()

‎e2e/prompts.spec.ts‎

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { expect } from '@playwright/test'
22
import { teacherTest as test } from './fixtures'
3-
import { acceptDisclaimer } from './utils/test-helpers'
3+
import { acceptDisclaimer, closeSendPreference, sendChatMessage } from './utils/test-helpers'
44

55
test.describe('Prompts', () => {
66
test('Custom prompt text works', async ({ page }) => {
@@ -25,9 +25,7 @@ test.describe('Prompts', () => {
2525
let chatInput = page.getByTestId('chat-input').first()
2626
await chatInput.fill('testinen morjens')
2727
await chatInput.press('Shift+Enter')
28-
29-
// Close send preference configurator
30-
await page.locator('#send-preference-configurator-submit').click()
28+
await closeSendPreference(page)
3129

3230
// The result should be echo of prompt
3331
await expect(page.getByTestId('assistant-message')).toContainText('mocktest testi onnistui')
@@ -81,9 +79,8 @@ test.describe('Prompts', () => {
8179
await page.keyboard.press('Escape')
8280

8381
// Send something
84-
const chatInput = page.getByTestId('chat-input').first()
85-
await chatInput.fill('testinen morjens')
86-
await chatInput.press('Shift+Enter')
82+
await sendChatMessage(page, 'testinen morjens')
83+
await closeSendPreference(page)
8784

8885
// The result should be echo of the course prompt
8986
await expect(page.getByTestId('assistant-message')).toContainText('mocktest kurssitesti onnistui')
@@ -147,12 +144,8 @@ test.describe('Prompts', () => {
147144
await page.keyboard.press('Escape')
148145

149146
// Send message, response should echo the prompt
150-
const chatInput = page.getByTestId('chat-input').first()
151-
await chatInput.fill('testinen morjens')
152-
await chatInput.press('Shift+Enter')
153-
154-
// Close send preference configurator
155-
await page.locator('#send-preference-configurator-submit').click()
147+
await sendChatMessage(page, 'testinen morjens')
148+
await closeSendPreference(page)
156149

157150
// Own prompt echoed:
158151
await expect(page.getByTestId('assistant-message')).toContainText(newPromptContent)

‎e2e/student.spec.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect } from '@playwright/test'
2-
import { acceptDisclaimer } from './utils/test-helpers'
2+
import { acceptDisclaimer, closeSendPreference, sendChatMessage } from './utils/test-helpers'
33
import { studentTest as test } from './fixtures'
44

55
test.describe('Student', () => {
@@ -10,9 +10,8 @@ test.describe('Student', () => {
1010
await page.getByTestId('model-selector').first().click()
1111
await page.getByRole('option', { name: 'mock' }).click()
1212

13-
const chatInput = page.getByTestId('chat-input').first()
14-
await chatInput.fill('testinen morjens')
15-
await chatInput.press('Shift+Enter')
13+
await sendChatMessage(page, 'testinen morjens')
14+
await closeSendPreference(page)
1615

1716
await expect(page.getByTestId('user-message')).toContainText('testinen morjens')
1817
await expect(page.getByTestId('assistant-message')).toContainText('You are calling mock endpoint for streaming mock data')

‎e2e/utils/test-helpers.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,13 @@ export const acceptDisclaimer = async (page: Page) => {
44
await page.getByTestId('accept-disclaimer').click()
55
await page.getByTestId('submit-accept-disclaimer').click()
66
}
7+
8+
export const closeSendPreference = async (page: Page) => {
9+
await page.getByTestId('submit-send-preference').click()
10+
}
11+
12+
export const sendChatMessage = async (page: Page, message: string) => {
13+
const chatInput = page.getByTestId('chat-input').first()
14+
await chatInput.fill(message)
15+
await chatInput.press('Shift+Enter')
16+
}

‎src/client/components/ChatV2/SendPreferenceConfigurator.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ export const SendPreferenceConfiguratorModal = ({ open, onClose, anchorEl, conte
124124
}
125125
/>
126126
</RadioGroup>
127-
<OutlineButtonBlack type="submit" id="send-preference-configurator-submit">
127+
<OutlineButtonBlack type="submit" data-testid="submit-send-preference">
128128
{t('sendPreferenceConfigurator:ok')}
129129
</OutlineButtonBlack>
130130
</FormControl>

‎src/client/components/Rag/RagIndex.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export const RagIndex: React.FC = () => {
5858
onClick={async () => {
5959
if (window.confirm(`Are you sure you want to delete index ${ragDetails.metadata?.name}?`)) {
6060
await deleteIndexMutation.mutateAsync(id)
61-
navigate('/rag')
61+
navigate(-1)
6262
}
6363
}}
6464
>

‎src/server/routes/rag/rag.ts‎

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import { Router } from 'express'
22
import z from 'zod/v4'
33
import { EMBED_DIM } from '../../../config'
4-
import { ChatInstance, ChatInstanceRagIndex, RagFile, RagIndex, Responsibility } from '../../db/models'
4+
import { ChatInstance, ChatInstanceRagIndex, Enrolment, RagFile, RagIndex, Responsibility } from '../../db/models'
55
import type { RequestWithUser } from '../../types'
6-
import type { User } from '../../../shared/user'
76
import { ApplicationError } from '../../util/ApplicationError'
87
import { TEST_COURSES } from '../../../shared/testData'
98
import ragIndexRouter, { ragIndexMiddleware } from './ragIndex'
9+
import { ChatInstanceAccess, getChatInstanceAccess } from '../../services/chatInstances/access'
1010

1111
const router = Router()
1212

@@ -17,22 +17,12 @@ const IndexCreationSchema = z.object({
1717
dim: z.number().min(EMBED_DIM).max(EMBED_DIM).default(EMBED_DIM),
1818
})
1919

20-
const hasChatInstanceRagPermission = (user: User, chatInstance: ChatInstance) => {
21-
const isResponsible = chatInstance.responsibilities?.some((r) => r.userId === user.id)
22-
return isResponsible || user.isAdmin
23-
}
24-
2520
router.post('/indices', async (req, res) => {
2621
const { user } = req as RequestWithUser
2722
const { name, dim, chatInstanceId, language } = IndexCreationSchema.parse(req.body)
2823

2924
const chatInstance = await ChatInstance.findByPk(chatInstanceId, {
3025
include: [
31-
{
32-
model: Responsibility,
33-
as: 'responsibilities',
34-
required: true, // Ensure the user is responsible for the course
35-
},
3626
{
3727
model: RagIndex,
3828
as: 'ragIndices',
@@ -44,7 +34,7 @@ router.post('/indices', async (req, res) => {
4434
throw ApplicationError.NotFound('Invalid chat instance id')
4535
}
4636

47-
if (!hasChatInstanceRagPermission(user, chatInstance)) {
37+
if ((await getChatInstanceAccess(user, chatInstance)) < ChatInstanceAccess.TEACHER) {
4838
throw ApplicationError.Forbidden('Cannot create index, user is not responsible for the course')
4939
}
5040

@@ -89,10 +79,7 @@ router.get('/indices', async (req, res) => {
8979
let chatInstance: ChatInstance | null = null
9080
if (chatInstanceId) {
9181
chatInstance = await ChatInstance.findByPk(chatInstanceId, {
92-
include: [
93-
{ model: Responsibility, as: 'responsibilities', required: !user.isAdmin },
94-
{ model: RagIndex, as: 'ragIndices', required: false },
95-
],
82+
include: [{ model: RagIndex, as: 'ragIndices', required: false }],
9683
})
9784

9885
if (!chatInstance) {
@@ -104,8 +91,8 @@ router.get('/indices', async (req, res) => {
10491
return
10592
}
10693

107-
if (!hasChatInstanceRagPermission(user, chatInstance)) {
108-
throw ApplicationError.Forbidden('Forbidden')
94+
if ((await getChatInstanceAccess(user, chatInstance)) < ChatInstanceAccess.STUDENT) {
95+
throw ApplicationError.Forbidden('Not allowed to use rag index. You must be at least an enrolled student.')
10996
}
11097
} else {
11198
if (!user.isAdmin) {

‎src/server/routes/testUtils.ts‎

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ router.post('/reset-test-data', async (req, res) => {
2121
const userId = testUserHeaders.hypersonsisuid
2222

2323
logger.info(`Resetting test data for user ${userId}`)
24+
2425
await Prompt.destroy({
2526
where: {
2627
userId,
@@ -53,35 +54,41 @@ router.post('/reset-test-data', async (req, res) => {
5354
})
5455

5556
await User.create(headersToUser(testUserHeaders))
56-
await Enrolment.create({
57-
userId,
58-
chatInstanceId: TEST_COURSES.TEST_COURSE.id,
59-
})
6057

61-
const [ragIndex] = await RagIndex.findOrCreate({
62-
where: {
63-
userId,
64-
},
65-
defaults: {
66-
userId,
67-
metadata: {
68-
name: `rag-${testUserIdx}`,
69-
language: 'English',
70-
},
71-
},
72-
})
73-
await ChatInstanceRagIndex.findOrCreate({
74-
where: {
58+
if (testUserRole === 'student') {
59+
await Enrolment.create({
7560
userId,
7661
chatInstanceId: TEST_COURSES.TEST_COURSE.id,
77-
ragIndexId: ragIndex.id,
78-
},
79-
defaults: {
80-
chatInstanceId: TEST_COURSES.TEST_COURSE.id,
81-
ragIndexId: ragIndex.id,
82-
userId,
83-
},
84-
})
62+
})
63+
}
64+
65+
if (testUserRole !== 'student') {
66+
const [ragIndex] = await RagIndex.findOrCreate({
67+
where: {
68+
userId,
69+
},
70+
defaults: {
71+
userId,
72+
metadata: {
73+
name: `rag-${testUserIdx}`,
74+
language: 'English',
75+
},
76+
},
77+
})
78+
await ChatInstanceRagIndex.findOrCreate({
79+
where: {
80+
userId,
81+
chatInstanceId: TEST_COURSES.TEST_COURSE.id,
82+
ragIndexId: ragIndex.id,
83+
},
84+
defaults: {
85+
chatInstanceId: TEST_COURSES.TEST_COURSE.id,
86+
ragIndexId: ragIndex.id,
87+
userId,
88+
},
89+
})
90+
}
91+
8592
logger.info('Test data reset successfully')
8693

8794
res.status(200).json({ message: 'Test data reset successfully' })

‎src/server/services/chatInstances/access.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,3 +80,39 @@ export const getOwnCourses = async (user: User) => {
8080

8181
return courseIds
8282
}
83+
84+
/**
85+
* @todo use this for authorization always.
86+
*/
87+
export const ChatInstanceAccess = {
88+
ADMIN: 3,
89+
TEACHER: 2,
90+
STUDENT: 1,
91+
NONE: 0,
92+
}
93+
94+
export const getChatInstanceAccess = async (user: User, chatInstance: ChatInstance) => {
95+
if (user.isAdmin) return ChatInstanceAccess.ADMIN
96+
97+
const [responsibilities, enrolments] = await Promise.all([
98+
Responsibility.findAll({
99+
attributes: ['id'],
100+
where: {
101+
userId: user.id,
102+
chatInstanceId: chatInstance.id,
103+
},
104+
}),
105+
Enrolment.findAll({
106+
attributes: ['id'],
107+
where: {
108+
userId: user.id,
109+
chatInstanceId: chatInstance.id,
110+
},
111+
}),
112+
])
113+
114+
if (responsibilities.length > 0) return ChatInstanceAccess.TEACHER
115+
if (enrolments.length > 0) return ChatInstanceAccess.STUDENT
116+
117+
return ChatInstanceAccess.NONE
118+
}

0 commit comments

Comments
 (0)