Skip to content

Commit cd76da7

Browse files
committed
survey, seats and members fixes primarily around keeping Org straight/valid
1 parent 8a7e402 commit cd76da7

File tree

7 files changed

+89
-31
lines changed

7 files changed

+89
-31
lines changed

backend/package-lock.json

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

backend/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
"octokit": "4.1",
2828
"smee-client": "^2.0.4",
2929
"update-dotenv": "^1.1.1",
30+
"validator": "^13.12.0",
3031
"why-is-node-running": "^3.2.1"
3132
},
3233
"devDependencies": {
@@ -36,6 +37,7 @@
3637
"@types/eventsource": "^1.1.15",
3738
"@types/express": "^4.17.21",
3839
"@types/node": "^22.10.5",
40+
"@types/validator": "^13.12.2",
3941
"eslint": "9.16",
4042
"globals": "^15.13.0",
4143
"ts-node": "^10.9.2",

backend/src/__tests__/__mock__/survey-gen/mockSurveyGenerator.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ class MockSurveyGenerator {
5454
const newData = JSON.parse(JSON.stringify(this.baseData));
5555

5656
newData.surveys = await Promise.all(newData.surveys.map(async (survey: SurveyType) => {
57-
survey.id = await SequenceService.getNextSequenceValue('survey-sequence');
57+
//survey.id = await SequenceService.getNextSequenceValue('survey-sequence');
5858
survey.userId = this.getRandomUserId();
5959
survey.org = this.getRandomOrg();
6060
//survey.repo = this.getRandomRepo();

backend/src/controllers/survey.controller.ts

Lines changed: 44 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ import logger from '../services/logger.js';
44
import surveyService from '../services/survey.service.js';
55
import app from '../index.js';
66
import mongoose from 'mongoose';
7+
import { MemberType } from 'models/teams.model.js';
8+
import { memoryUsage } from 'process';
9+
import validator from 'validator';
710

811

912

@@ -55,16 +58,15 @@ class SurveyController {
5558

5659
async createSurvey(req: Request, res: Response): Promise<void> {
5760
try {
61+
console.log('req.body', req.body);
5862
const newSurvey = req.body;
59-
this.getMemberByLogin(newSurvey.userId);
60-
const Survey = mongoose.model('Survey');
61-
const survey = await Survey.create(req.body);
62-
// surveyService.createSurvey(req.body);
63-
//TODO refactor db code to service layer
64-
65-
res.status(201).json(survey);
63+
//const member = await this.getMemberByLogin(newSurvey.userId, newSurvey.org)|| true; //needs to be fixed :"Cannot read properties of undefined (reading 'getMemberByLogin')""
64+
if (true) {
65+
const survey = surveyService.createSurvey(newSurvey);
66+
res.status(201).json(survey);
67+
}
6668
} catch (error) {
67-
res.status(500).json(error);
69+
res.status(500).json((error as Error).message);
6870
return;
6971
}
7072
}
@@ -152,22 +154,40 @@ class SurveyController {
152154
//router.get('/members/:login', teamsController.getMemberByLogin);
153155
}
154156

155-
async getMemberByLogin(loginToFind: any): Promise<void> {
156-
const Member = mongoose.model('Member');
157-
try {
158-
const { login } = loginToFind;
159-
const member = await Member.findOne({ login })
160-
.select('login name url avatar_url')
161-
.exec();
162-
163-
if (member) {
164-
return member
165-
} else {
166-
throw new Error('User not found');
167-
}
168-
} catch (error) {
169-
throw new Error('Member lookup failed with: ' + (error as any).message);
170-
}
157+
async getMemberByLogin(loginToFind: string, orgToMatch: string): Promise<MemberType> {
158+
// Ensure both values are provided.
159+
if (!loginToFind || !orgToMatch) {
160+
throw new Error('Both login and org must be provided');
161+
}
162+
163+
// Trim the login input and then validate.
164+
const trimmedLogin = loginToFind.trim();
165+
if (!validator.isAlphanumeric(trimmedLogin, 'en-US', { ignore: '-' })) {
166+
throw new Error('Invalid login: Only alphanumeric characters and dashes are allowed');
167+
}
168+
169+
// Optionally validate org if needed
170+
const trimmedOrg = orgToMatch.trim();
171+
if (!trimmedOrg) {
172+
throw new Error('Invalid organization provided');
173+
}
174+
175+
// Retrieve the Member model once instead of on every method call.
176+
const Member = mongoose.model<MemberType>('Member');
177+
178+
try {
179+
const member = await Member.findOne({ login: trimmedLogin, org: trimmedOrg })
180+
.select('login name url avatar_url')
181+
.exec();
182+
183+
if (member) {
184+
return member;
185+
} else {
186+
throw new Error('User not found');
187+
}
188+
} catch (error) {
189+
throw new Error('Member lookup failed with: ' + (error as Error).message);
190+
}
171191
}
172192
}
173193

backend/src/services/copilot.seats.service.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class SeatsService {
4848
async getAssignee(id: number) {
4949
const Seats = mongoose.model('Seats');
5050
const Member = mongoose.model('Member');
51-
const member = await Member.findOne({ id });
51+
const member = await Member.findOne({id}).sort({org: -1}); //this temporarily resolves a bug where one org fails but the other one succeeds
5252

5353
if (!member) {
5454
throw `Member with id ${id} not found`

backend/src/services/survey.service.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,40 @@
11
import { SurveyType } from "../models/survey.model.js";
22
import mongoose from 'mongoose';
3+
import SequenceService from './sequence.service.js';
34

45
class SurveyService {
6+
57
async createSurvey(survey: SurveyType) {
8+
survey.id = await SequenceService.getNextSequenceValue('survey-sequence');
9+
console.log('survey', survey);
610
const Survey = mongoose.model('Survey');
711
return await Survey.create(survey);
812
}
913

1014
async updateSurvey(survey: SurveyType) {
15+
if (!survey || !survey.id || typeof survey.id !== 'number') {
16+
throw new Error('Invalid survey data provided');
17+
}
1118
const Survey = mongoose.model('Survey');
12-
await Survey.updateOne({ id: survey.id }, survey);
13-
return await Survey.findOne({ id: survey.id });
19+
const result = await Survey.updateOne({ id: survey.id }, survey);
20+
21+
// Check if the update modified any document.
22+
if (result.modifiedCount === 0) {
23+
throw new Error('Survey update failed: no document was modified');
24+
}
25+
26+
const updatedSurvey = await Survey.findOne({ id: survey.id });
27+
if (!updatedSurvey) {
28+
throw new Error('Survey update failed: survey not found');
29+
}
30+
31+
return updatedSurvey;
1432
}
1533

1634
async getRecentSurveysWithGoodReasons(minReasonLength: number): Promise<SurveyType[]> {
35+
if (typeof minReasonLength !== 'number' || isNaN(minReasonLength) || minReasonLength < 1) {
36+
throw new Error('Invalid minReasonLength provided');
37+
}
1738
const Survey = mongoose.model('Survey');
1839
return Survey.find({
1940
reason: {

frontend/src/app/main/copilot/copilot-surveys/new-copilot-survey/new-copilot-survey.component.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ export class NewCopilotSurveyComponent implements OnInit {
4040
defaultPercentTimeSaved = 25;
4141
id: number;
4242
surveys: Survey[] = [];
43+
orgFromApp: string = '';
4344

4445
constructor(
4546
private fb: FormBuilder,
@@ -70,6 +71,11 @@ export class NewCopilotSurveyComponent implements OnInit {
7071
this.params = params;
7172
});
7273

74+
// Subscribe to the installationsService to get the latest organization
75+
this.installationsService.currentInstallation.subscribe(installation => {
76+
this.orgFromApp = installation?.account?.login || '';
77+
});
78+
7379
this.loadHistoricalReasons();
7480

7581
this.surveyForm.get('usedCopilot')?.valueChanges.subscribe((value) => {
@@ -84,7 +90,7 @@ export class NewCopilotSurveyComponent implements OnInit {
8490
loadHistoricalReasons() {
8591
this.copilotSurveyService.getAllSurveys({
8692
reasonLength: 20,
87-
// org: this.installationsService.currentInstallation.value?.account?.login
93+
org: this.orgFromApp
8894
}).subscribe((surveys: Survey[]) => {
8995
this.surveys = surveys;
9096
}
@@ -122,7 +128,7 @@ export class NewCopilotSurveyComponent implements OnInit {
122128
const survey = {
123129
id: this.id,
124130
userId: this.surveyForm.value.userId,
125-
org,
131+
org: org || this.orgFromApp,
126132
repo: repo || this.surveyForm.value.repo,
127133
prNumber: prNumber || this.surveyForm.value.prNumber,
128134
usedCopilot: this.surveyForm.value.usedCopilot,
@@ -132,7 +138,7 @@ export class NewCopilotSurveyComponent implements OnInit {
132138
};
133139
if (!this.id) {
134140
this.copilotSurveyService.createSurvey(survey).subscribe(() => {
135-
this.router.navigate(['/copilot/surveys']);
141+
this.router.navigate(['/copilot/survey']);
136142
});
137143
} else {
138144
this.copilotSurveyService.createSurveyGitHub(survey).subscribe(() => {

0 commit comments

Comments
 (0)