Skip to content

Commit

Permalink
fix: Minor refactor to fix the verify role check and also the naming …
Browse files Browse the repository at this point in the history
…of the discord service function to get roles
  • Loading branch information
Maelstromeous committed Aug 8, 2024
1 parent 480848b commit 01fff5f
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 24 deletions.
24 changes: 12 additions & 12 deletions src/albion/services/albion.registration.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ describe('AlbionRegistrationService', () => {
albionApiService = moduleRef.get<AlbionApiService>(AlbionApiService);

// Mocks
discordService.getMemberRole = jest.fn().mockReturnValue(TestBootstrapper.getMockDiscordRole('123456789'));
discordService.getRoleViaMember = jest.fn().mockReturnValue(TestBootstrapper.getMockDiscordRole('123456789'));

// Fragments
contactMessageUS = `If you believe this to be in error, please contact \`${mockRegistrationData.guildPingable}\` in <#1039269706605002912>.`;
Expand Down Expand Up @@ -173,7 +173,7 @@ describe('AlbionRegistrationService', () => {
describe('validate', () => {
describe('checkRolesExist', () => {
it('should throw an error if one of the roles is missing ', async () => {
discordService.getMemberRole = jest.fn()
discordService.getRoleViaMember = jest.fn()
.mockReturnValueOnce({
id: mockAlbionUSMemberRoleId,
})
Expand Down Expand Up @@ -374,9 +374,9 @@ describe('AlbionRegistrationService', () => {
TestBootstrapper.getMockDiscordTextChannel().id,
);

expect(discordService.getMemberRole).toHaveBeenCalledWith(mockRegistrationData.discordMember, TestBootstrapper.mockConfig.discord.roles.albionUSMember);
expect(discordService.getMemberRole).toHaveBeenCalledWith(mockRegistrationData.discordMember, TestBootstrapper.mockConfig.discord.roles.albionUSRegistered);
expect(discordService.getMemberRole).toHaveBeenCalledWith(mockRegistrationData.discordMember, TestBootstrapper.mockConfig.discord.roles.albionUSAnnouncements);
expect(discordService.getRoleViaMember).toHaveBeenCalledWith(mockRegistrationData.discordMember, TestBootstrapper.mockConfig.discord.roles.albionUSMember);
expect(discordService.getRoleViaMember).toHaveBeenCalledWith(mockRegistrationData.discordMember, TestBootstrapper.mockConfig.discord.roles.albionUSRegistered);
expect(discordService.getRoleViaMember).toHaveBeenCalledWith(mockRegistrationData.discordMember, TestBootstrapper.mockConfig.discord.roles.albionUSAnnouncements);
expect(mockRegistrationData.discordMember.roles.add).toHaveBeenCalledTimes(3);
});
it('should add the correct number of roles, EU', async () => {
Expand All @@ -392,9 +392,9 @@ describe('AlbionRegistrationService', () => {
TestBootstrapper.getMockDiscordTextChannel().id,
);

expect(discordService.getMemberRole).toHaveBeenCalledWith(mockRegistrationDataEU.discordMember, TestBootstrapper.mockConfig.discord.roles.albionMember);
expect(discordService.getMemberRole).toHaveBeenCalledWith(mockRegistrationDataEU.discordMember, TestBootstrapper.mockConfig.discord.roles.albionRegistered);
expect(discordService.getMemberRole).toHaveBeenCalledWith(mockRegistrationDataEU.discordMember, TestBootstrapper.mockConfig.discord.roles.albionAnnouncements);
expect(discordService.getRoleViaMember).toHaveBeenCalledWith(mockRegistrationDataEU.discordMember, TestBootstrapper.mockConfig.discord.roles.albionMember);
expect(discordService.getRoleViaMember).toHaveBeenCalledWith(mockRegistrationDataEU.discordMember, TestBootstrapper.mockConfig.discord.roles.albionRegistered);
expect(discordService.getRoleViaMember).toHaveBeenCalledWith(mockRegistrationDataEU.discordMember, TestBootstrapper.mockConfig.discord.roles.albionAnnouncements);
expect(mockRegistrationDataEU.discordMember.roles.add).toHaveBeenCalledTimes(3);
});
it('should handle discord role adding errors', async () => {
Expand All @@ -417,7 +417,7 @@ describe('AlbionRegistrationService', () => {
it('should throw upon database error', async () => {
service.getInfo = jest.fn().mockResolvedValue(mockRegistrationDataEU);
service.validate = jest.fn().mockResolvedValue(undefined);
discordService.getMemberRole = jest.fn().mockReturnValue({
discordService.getRoleViaMember = jest.fn().mockReturnValue({
id: mockAlbionUSMemberRoleId,
});
mockAlbionRegistrationsRepository.upsert = jest.fn().mockImplementation(() => {
Expand All @@ -435,7 +435,7 @@ describe('AlbionRegistrationService', () => {
it('should handle discord nickname permission errors by sending a message only', async () => {
service.getInfo = jest.fn().mockResolvedValue(mockRegistrationDataEU);
service.validate = jest.fn().mockResolvedValue(undefined);
discordService.getMemberRole = jest.fn().mockReturnValue({
discordService.getRoleViaMember = jest.fn().mockReturnValue({
id: mockAlbionUSMemberRoleId,
});
const mockChannel = TestBootstrapper.getMockDiscordTextChannel();
Expand Down Expand Up @@ -464,7 +464,7 @@ describe('AlbionRegistrationService', () => {
it('should handle successful US registration and return a message to the user', async () => {
service.getInfo = jest.fn().mockResolvedValue(mockRegistrationData);
service.validate = jest.fn().mockImplementation(() => true);
discordService.getMemberRole = jest.fn().mockReturnValue({
discordService.getRoleViaMember = jest.fn().mockReturnValue({
id: mockAlbionUSMemberRoleId,
});
mockDiscordUser.roles.add = jest.fn().mockReturnValue(true);
Expand Down Expand Up @@ -498,7 +498,7 @@ CC <@&${mockUSLeaderRoleId}>, <@&${mockUSOfficerRoleId}>`,
it('should handle successful EU registration and return a message to the user', async () => {
service.getInfo = jest.fn().mockResolvedValue(mockRegistrationDataEU);
service.validate = jest.fn().mockImplementation(() => true);
discordService.getMemberRole = jest.fn().mockReturnValue({
discordService.getRoleViaMember = jest.fn().mockReturnValue({
id: mockAlbionUSMemberRoleId,
});
mockDiscordUser.roles.add = jest.fn().mockReturnValue(true);
Expand Down
13 changes: 6 additions & 7 deletions src/albion/services/albion.registration.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,15 +121,14 @@ export class AlbionRegistrationService implements OnApplicationBootstrap {

private async checkRolesExist(data: RegistrationData) {
const rolesToCheck = [
this.config.get('discord.roles.albionUSMember'),
this.config.get('discord.roles.albionUSRegistered'),
this.config.get('discord.roles.albionMember'),
this.config.get('discord.roles.albionRegistered'),
this.config.get('discord.roles.albionUSAnnouncements'),
this.config.get('discord.roles.albionAnnouncements'),
];

try {
for (const roleId of rolesToCheck) {
await this.discordService.getMemberRole(data.discordMember, roleId);
await this.discordService.getRoleViaMember(data.discordMember, roleId);
}
}
catch (err) {
Expand Down Expand Up @@ -219,15 +218,15 @@ export class AlbionRegistrationService implements OnApplicationBootstrap {
const announcementRole = data.server === AlbionServer.AMERICAS ? this.config.get('discord.roles.albionUSAnnouncements') : this.config.get('discord.roles.albionAnnouncements');

try {
await data.discordMember.roles.add(await this.discordService.getMemberRole(
await data.discordMember.roles.add(await this.discordService.getRoleViaMember(
data.discordMember,
memberRole
));
await data.discordMember.roles.add(await this.discordService.getMemberRole(
await data.discordMember.roles.add(await this.discordService.getRoleViaMember(
data.discordMember,
registeredRole
));
await data.discordMember.roles.add(await this.discordService.getMemberRole(
await data.discordMember.roles.add(await this.discordService.getRoleViaMember(
data.discordMember,
announcementRole
));
Expand Down
6 changes: 3 additions & 3 deletions src/discord/discord.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ describe('DiscordService', () => {
// Mock the get method of guilds cache
mockDiscordClient.guilds.cache.get = jest.fn().mockReturnValue(mockGuild);

const result = await service.getMemberRole(guildMember, roleId);
const result = await service.getRoleViaMember(guildMember, roleId);

expect(result.id).toEqual(mockRole.id);
expect(mockGuild.roles.fetch).toHaveBeenCalledWith(roleId);
Expand All @@ -164,7 +164,7 @@ describe('DiscordService', () => {
// Mock the get method of guilds cache
mockDiscordClient.guilds.cache.get = jest.fn().mockReturnValue(mockGuild);

await expect(service.getMemberRole(guildMember, roleId)).rejects.toThrow(`Failed to fetch role with ID ${roleId}. Err: ${fetchError.message}`);
await expect(service.getRoleViaMember(guildMember, roleId)).rejects.toThrow(`Failed to fetch role with ID ${roleId}. Err: ${fetchError.message}`);
expect(mockGuild.roles.fetch).toHaveBeenCalledWith(roleId);
});

Expand All @@ -177,7 +177,7 @@ describe('DiscordService', () => {
// Mock the get method of guilds cache
mockDiscordClient.guilds.cache.get = jest.fn().mockReturnValue(mockGuild);

await expect(service.getMemberRole(guildMember, roleId)).rejects.toThrow(`Could not find role with ID ${roleId}`);
await expect(service.getRoleViaMember(guildMember, roleId)).rejects.toThrow(`Could not find role with ID ${roleId}`);
expect(mockGuild.roles.fetch).toHaveBeenCalledWith(roleId);
});
});
Expand Down
2 changes: 1 addition & 1 deletion src/discord/discord.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export class DiscordService {
return member;
}

async getMemberRole(guildMember: GuildMember, roleId: string): Promise<Role> {
async getRoleViaMember(guildMember: GuildMember, roleId: string): Promise<Role> {
const serverId = guildMember.guild.id;

let role: Role;
Expand Down
2 changes: 1 addition & 1 deletion src/ps2/service/ps2.game.verification.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ export class PS2GameVerificationService implements OnApplicationBootstrap {
private async applyDiscordChanges(character: CensusCharacterWithOutfitInterface, guildMember: GuildMember, remove = false) {
// Add the PS2/verified role to the Discord user
const verifiedRoleId = this.config.get('discord.roles.ps2Verified');
const verifyRole = await this.discordService.getMemberRole(guildMember, verifiedRoleId);
const verifyRole = await this.discordService.getRoleViaMember(guildMember, verifiedRoleId);

if (!verifyRole) {
throw new Error(`Could not find role with ID ${verifiedRoleId}`);
Expand Down

0 comments on commit 01fff5f

Please sign in to comment.