Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: refactored participant request (team, member) service #1936

Merged
merged 1 commit into from
Nov 3, 2024

Conversation

navneethkrish
Copy link
Contributor

Description

Add a few lines describing your changes

Tickets


Checklist before requesting a review

  • I've performed a self-review of my code
  • I've added relevant tests for my changes
  • I've confirmed that my code passes linting
  • I've confirmed that my code passes all tests
  • I've confirmed that my code builds correctly

Copy link

vercel bot commented Oct 22, 2024

@navneethkrish is attempting to deploy a commit to the Protocol Labs Spaceport team on Vercel, but is not a member of this team. To resolve this issue, you can:

  • Make your repository public. Collaboration is free for open source and public repositories.
  • Add @navneethkrish as a member. A Pro subscription is required to access Vercel's collaborative features.
    • If you're the owner of the team, click here and add @navneethkrish as a member.
    • If you're the user who initiated this build request, click here to request access.
    • If you're already a member of the Protocol Labs Spaceport team, make sure that your Vercel account is connected to your GitHub account.

To read more about collaboration on Vercel, click here.

@navneethkrish navneethkrish force-pushed the feat/participant-request-refactor branch 5 times, most recently from 51e6f55 to 0cb2058 Compare October 23, 2024 09:36
@navneethkrish navneethkrish force-pushed the develop branch 2 times, most recently from 3253319 to cda4f01 Compare October 23, 2024 09:56
@navneethkrish navneethkrish force-pushed the feat/participant-request-refactor branch from 0cb2058 to 5dd455d Compare October 23, 2024 10:44
@navneethkrish navneethkrish force-pushed the feat/participant-request-refactor branch 4 times, most recently from cbb4e18 to 7351eaf Compare October 29, 2024 09:08
imports: [CacheModule.register()],
controllers: [AdminController],
imports: [CacheModule.register(), ParticipantsRequestModule, SharedModule],
controllers: [AdminParticipantsRequestController, AdminAuthController],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name can be simply ParticipantsRequestController

}
console.log('generating token.....');
this.logger.info('Generating admin access token...');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can add some context like username

): Promise<any> {
const participantRequest: ParticipantsRequest | null = await this.participantsRequestService.findOneByUid(uid);
if (!participantRequest) {
throw new NotFoundException('Request not found');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change as Participant Request no found

!requestor.isDirectoryAdmin &&
referenceUid !== requestor.uid
) {
throw new ForbiddenException(`Member isn't authorized to update the member`);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we implement/reuse any guards for this at the controller level?

}
let isMemberAvailable = await this.membersService.isMemberExistForEmailId(oldEmailId);
if (!isMemberAvailable) {
throw new ForbiddenException('Your email seems to have been updated recently. Please login and try again');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please login and try again with your updated email

* @param member - The member object.
* @returns True if the member is a directory admin, false otherwise.
*/
checkIfAdminUser(member): boolean {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isMemberAdmin

* @param tx - Optional transaction client.
* @returns Updated member object if a change was made, otherwise the original member object.
*/
async updateTelegramIfChanged(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You already have updateFieldIfChanged - can we resuse that?

* @param tx - Optional transaction client.
* @returns Updated member object if a change was made, otherwise the original member object.
*/
async updateOfficeHoursIfChanged(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment as telegram

});
} else if (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why slack config logic removed?

@@ -13,7 +13,7 @@ export class SetupService {
winston.format.timestamp(),
winston.format.ms(),
winston.format.printf((info) => {
return `${info.timestamp} : ${info.level} - ${info.message}`;
return `${JSON.stringify(info)}`;;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

@navneethkrish navneethkrish force-pushed the feat/participant-request-refactor branch from 7351eaf to 642f9be Compare November 3, 2024 10:17
@navneethkrish navneethkrish merged commit 5f74020 into develop Nov 3, 2024
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants