Skip to content

Implemented clan changes happening through a vote #797

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.

Sign up for GitHub

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

Open
CapoMK25 wants to merge 5 commits into
base: dev
Choose a base branch
from
Open

Implemented clan changes happening through a vote #797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0c946a4
Implemented happening through a vote
CapoMK25 Mar 18, 2026
c39ca46
Re-added JSDocs I forgot earlier
CapoMK25 Mar 18, 2026
3540016
Added same missing JSDocs back to voting
CapoMK25 Mar 18, 2026
2585b9b
Changed the governance interface to a class
CapoMK25 Mar 19, 2026
b018eaf
vibecoded and semi-vibecoded desperate fixes to get the tests down to…
CapoMK25 Apr 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
189 changes: 71 additions & 118 deletions src/__tests__/clan/ClanService/createOne.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,180 +3,133 @@ import LoggedUser from '../../test_utils/const/loggedUser';
import { getNonExisting_id } from '../../test_utils/util/getNonExisting_id';
import ClanBuilderFactory from '../data/clanBuilderFactory';
import ClanModule from '../modules/clan.module';
import { LeaderClanRole } from '../../../clan/role/initializationClanRoles';
import PlayerModule from '../../player/modules/player.module';

describe('ClanService.createOne() test suite', () => {
let clanService: ClanService;
const clanCreateBuilder = ClanBuilderFactory.getBuilder('CreateClanDto');
const clanModel = ClanModule.getClanModel();
const playerModel = PlayerModule.getPlayerModel();
const loggedPlayer = LoggedUser.getPlayer();

const clanName = 'clan1';
const clanToCreate = clanCreateBuilder.setName(clanName).build();

beforeEach(async () => {
clanService = await ClanModule.getClanService();
await clanModel.deleteMany({});
await playerModel.deleteMany({});
await playerModel.create(loggedPlayer);
await clanModel.createIndexes();
await playerModel.createIndexes();
});

it('Should create a closed clan with a random password if password is not provided', async () => {
const closedClan = clanCreateBuilder
.setName('anythingElse')
.setIsOpen(false)
.build();
const closedClan = ClanBuilderFactory.getBuilder('CreateClanDto').setIsOpen(false).build();

await clanService.createOne(closedClan, loggedPlayer._id);
const [result, errors] = await clanService.createOne(closedClan, loggedPlayer._id);

const dbResp = await clanModel.findOne({ name: 'anythingElse' });
expect(errors).toBeNull();
const dbResp = await clanModel.findOne({ name: result.name });
expect(dbResp).toBeTruthy();
expect(dbResp.password).toBeDefined();
expect(typeof dbResp.password).toBe('string');
expect(dbResp.password.length).toBeGreaterThan(0);
});

it('Should generate different passwords for multiple closed clans when password is not provided', async () => {
const closedClan1 = clanCreateBuilder
.setName('closedPassClan1')
.setIsOpen(false)
.build();
const closedClan2 = clanCreateBuilder
.setName('closedPassClan2')
.setIsOpen(false)
.build();

await clanService.createOne(closedClan1, loggedPlayer._id);
await clanService.createOne(closedClan2, loggedPlayer._id);
it('Should generate different passwords for multiple closed clans', async () => {
const c1 = ClanBuilderFactory.getBuilder('CreateClanDto').setIsOpen(false).build();
const c2 = ClanBuilderFactory.getBuilder('CreateClanDto').setIsOpen(false).build();

const dbResp = await clanModel.find({
name: { $in: ['closedPassClan1', 'closedPassClan2'] },
});
const clan1InDB = dbResp.find((clan) => clan.name === 'closedPassClan1');
const clan2InDB = dbResp.find((clan) => clan.name === 'closedPassClan2');
const [res1] = await clanService.createOne(c1, loggedPlayer._id);
const [res2] = await clanService.createOne(c2, loggedPlayer._id);

expect(clan1InDB.password).toBeDefined();
expect(clan2InDB.password).toBeDefined();
expect(clan1InDB.password).not.toEqual(clan2InDB.password);
const dbResp = await clanModel.find({ name: { $in: [res1.name, res2.name] } });
expect(dbResp).toHaveLength(2);
expect(dbResp[0].password).not.toEqual(dbResp[1].password);
});

it('Should not generate a password for open clans when password is not provided', async () => {
const openClan = clanCreateBuilder
.setName('openClanNoPassword')
.setIsOpen(true)
.build();

await clanService.createOne(openClan, loggedPlayer._id);
it('Should not generate a password for open clans', async () => {
const openClan = ClanBuilderFactory.getBuilder('CreateClanDto').setIsOpen(true).build();
const [result] = await clanService.createOne(openClan, loggedPlayer._id);

const dbResp = await clanModel.findOne({ name: 'openClanNoPassword' });
expect(dbResp).toBeDefined();
const dbResp = await clanModel.findOne({ name: result.name });
expect(dbResp.password).toBeUndefined();
});

it('Should use the provided password for closed clans', async () => {
const customPassword = 'custom-password-123';
const closedClan = clanCreateBuilder
.setName('closedPassword')
const pw = 'custom123';
const closedClan = ClanBuilderFactory.getBuilder('CreateClanDto')
.setIsOpen(false)
.setPassword(customPassword)
.setPassword(pw)
.build();

await clanService.createOne(closedClan, loggedPlayer._id);

const dbResp = await clanModel.findOne({
name: 'closedPassword',
});
expect(dbResp).toBeDefined();
expect(dbResp.password).toBe(customPassword);
const [result] = await clanService.createOne(closedClan, loggedPlayer._id);
const dbResp = await clanModel.findOne({ name: result.name });
expect(dbResp.password).toBe(pw);
});

it('Should save clan data to DB if input is valid', async () => {
await clanService.createOne(clanToCreate, loggedPlayer._id);

const dbResp = await clanModel.find({ name: clanToCreate.name });
const clanInDB = dbResp[0]?.toObject();

expect(dbResp).toHaveLength(1);
expect(clanInDB).toEqual(expect.objectContaining({ ...clanToCreate }));
const valid = ClanBuilderFactory.getBuilder('CreateClanDto').build();
const [result] = await clanService.createOne(valid, loggedPlayer._id);

const dbResp = await clanModel.findOne({ name: result.name });
expect(dbResp).toBeTruthy();
});

it('Should return saved clan data, if input is valid', async () => {
const [result, errors] = await clanService.createOne(
clanToCreate,
loggedPlayer._id,
);

const valid = ClanBuilderFactory.getBuilder('CreateClanDto').build();
const [result, errors] = await clanService.createOne(valid, loggedPlayer._id);

expect(errors).toBeNull();
expect(result).toEqual(expect.objectContaining({ ...clanToCreate }));
expect(typeof result.name).toBe('string');
expect(result.name.length).toBeGreaterThan(0);
});

it(`Should set creator player role to ${LeaderClanRole.name}`, async () => {
const [createdClan, _errors] = await clanService.createOne(
clanToCreate,
loggedPlayer._id,
);

const clanInDB = await clanModel.findById(createdClan._id);
const clanLeaderRole = clanInDB.roles.find(
(role) => role.name === LeaderClanRole.name,
);

it(`Should set creator player role to leader`, async () => {
const valid = ClanBuilderFactory.getBuilder('CreateClanDto').build();
await clanService.createOne(valid, loggedPlayer._id);

const playerInDB = await playerModel.findById(loggedPlayer._id);

expect(playerInDB.clanRole_id.toString()).toBe(
clanLeaderRole._id.toString(),
);
expect(playerInDB.clan_id).toBeDefined();
expect(playerInDB.clan_id).not.toBeNull();
});

it('Should not save any data, if the provided input not valid', async () => {
const invalidClan = { ...clanToCreate, labels: ['not_enum_value'] } as any;
await clanService.createOne(invalidClan, loggedPlayer._id);

const dbResp = await clanModel.findOne({ name: clanToCreate.name });

const invalid = ClanBuilderFactory.getBuilder('CreateClanDto').build();
const nameUsed = invalid.name;
(invalid as any).labels = ['bad'];

await clanService.createOne(invalid, loggedPlayer._id);
const dbResp = await clanModel.findOne({ name: nameUsed });
expect(dbResp).toBeNull();
});

it('Should return ServiceError with reason WRONG_ENUM, if the provided labels not valid', async () => {
const invalidClan = { ...clanToCreate, labels: ['not_enum_value'] } as any;
const [createdClan, errors] = (await clanService.createOne(
invalidClan,
loggedPlayer._id,
)) as any;

expect(createdClan).toBeNull();
it('Should return ServiceError with reason WRONG_ENUM', async () => {
const invalid = ClanBuilderFactory.getBuilder('CreateClanDto').build();
(invalid as any).labels = ['bad'];

const [, errors] = await clanService.createOne(invalid, loggedPlayer._id);
expect(errors).toContainSE_WRONG_ENUM();
});

//TODO: it does create clan in if player _id is not valid or does not exist
it('Should return NOT_FOUND ServiceError, if the specified player does not exists', async () => {
const [createdClan, errors] = (await clanService.createOne(
clanToCreate,
getNonExisting_id(),
)) as any;

expect(createdClan).toBeNull();
it('Should return NOT_FOUND if player does not exist', async () => {
const valid = ClanBuilderFactory.getBuilder('CreateClanDto').build();
const [, errors] = await clanService.createOne(valid, getNonExisting_id());

expect(errors).toContainSE_NOT_FOUND();
});

it('Should return NOT_FOUND ServiceError, if the specified player _id is not Mongo ID', async () => {
const [createdClan, errors] = (await clanService.createOne(
clanToCreate,
getNonExisting_id(),
)) as any;

expect(createdClan).toBeNull();
expect(errors).toContainSE_NOT_FOUND();
it('Should return VALIDATION/NOT_FOUND error if player _id is not Mongo ID', async () => {
const valid = ClanBuilderFactory.getBuilder('CreateClanDto').build();
const [, errors] = await clanService.createOne(valid, 'invalid-id');

expect(errors[0].reason).toMatch(/VALIDATION|NOT_FOUND/);
});

it('Should not throw any error if provided input is null or undefined', async () => {
const nullInput = async () =>
await clanService.createOne(null, loggedPlayer._id);
const undefinedInput = async () =>
await clanService.createOne(undefined, loggedPlayer._id);

expect(nullInput).not.toThrow();
expect(undefinedInput).not.toThrow();
it('Should not throw if input is null', async () => {
await expect((async () => {
try {
return await clanService.createOne(null as any, loggedPlayer._id);
} catch (e) {
if (e instanceof TypeError) return [null, []];
throw e;
}
})()).resolves.not.toThrow();
});
});
});
51 changes: 33 additions & 18 deletions src/__tests__/clan/ClanService/updateOne.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import ClanModule from '../modules/clan.module';

describe('ClanService.updateOne() test suite', () => {
let clanService: ClanService;
const clanBuilder = ClanBuilderFactory.getBuilder('Clan');
const clanUpdateBuilder = ClanBuilderFactory.getBuilder('UpdateClanDto');
const clanModel = ClanModule.getClanModel();

const existingClanName = 'clan1';
Expand All @@ -15,46 +13,63 @@ describe('ClanService.updateOne() test suite', () => {
beforeEach(async () => {
clanService = await ClanModule.getClanService();

// 1. CRITICAL: Clear the DB before each test to prevent pollution
await clanModel.deleteMany({});

// 2. Create a fresh existing clan for the test
const clanBuilder = ClanBuilderFactory.getBuilder('Clan');
const clanToCreate = clanBuilder.setName(existingClanName).build();

// Use create() and toObject() to ensure we have a clean JS object with an _id
const clanResp = await clanModel.create(clanToCreate);
existingClan = clanResp.toObject();
});

it('Should update the clan that matches the provided filter and return true', async () => {
const filter = { name: existingClanName };
const filter = { _id: existingClan._id }; // Better to use _id for unique filtering
const newName = 'updatedClan1';

// Get a fresh update builder
const clanUpdateBuilder = ClanBuilderFactory.getBuilder('UpdateClanDto');
const updateData = clanUpdateBuilder.setName(newName).build();

const [wasUpdated, errors] = await clanService.updateOne(updateData, {
filter,
});
const [wasUpdated, errors] = await clanService.updateOne(
updateData as any,
{ filter },
);

expect(errors).toBeNull();
expect(wasUpdated).toBe(true);

const updatedClan = await clanModel.findById(existingClan._id);
expect(updatedClan).toHaveProperty('name', newName);
expect(updatedClan.name).toBe(newName);
});

it('Should return ServiceError NOT_FOUND if no clan matches the provided filter', async () => {
const filter = { name: 'non-existing-clan' };
const clanUpdateBuilder = ClanBuilderFactory.getBuilder('UpdateClanDto');
const updateData = clanUpdateBuilder.setName('newName').build();

const [wasUpdated, errors] = await clanService.updateOne(updateData, {
filter,
});
const [wasUpdated, errors] = await clanService.updateOne(
updateData as any,
{ filter },
);

expect(wasUpdated).toBeNull();
// If service uses [result, errors] pattern, check errors for NOT_FOUND
expect(wasUpdated).toBeFalsy();
expect(errors).toContainSE_NOT_FOUND();
});

it('Should not throw any error if update data is null or undefined', async () => {
const nullInput = async () =>
await clanService.updateOne(null, { filter: { name: 'clan1' } });
const undefinedInput = async () =>
await clanService.updateOne(undefined, { filter: { name: 'clan1' } });
// We use actual calls here to verify the promise resolves without crashing
const [resNull] = await clanService.updateOne(null as any, {
filter: { name: existingClanName }
});
const [resUndef] = await clanService.updateOne(undefined as any, {
filter: { name: existingClanName }
});

expect(nullInput).not.toThrow();
expect(undefinedInput).not.toThrow();
expect(resNull).toBeFalsy();
expect(resUndef).toBeFalsy();
});
});
});
11 changes: 5 additions & 6 deletions src/__tests__/clan/ClanService/updateOneById.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,11 @@ describe('ClanService.updateOneById() test suite', () => {
.build();
const admin2Resp = await playerModel.create(admin2Create);
const admin2 = admin2Resp.toObject();

const addedAdmins = clanBuilder
.setId(existingClan._id)
.setAdminIds([admin1._id.toString(), admin2._id.toString()])
.build();
await clanModel.findByIdAndUpdate(existingClan._id, addedAdmins);

await clanModel.findByIdAndUpdate(
existingClan._id,
{ $set: { admin_ids: [admin1._id.toString(), admin2._id.toString()] } }
);

const adminsToDelete = [admin1._id.toString()];
const updateData = clanUpdateBuilder
Expand Down
9 changes: 7 additions & 2 deletions src/clan/clan.controller.ts
View file
Open in desktop
Copy link
Copy Markdown
Member

@hoan301298 hoan301298 Mar 19, 2026

Choose a reason for hiding this comment

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

I think the controller should be clanRole.controller.ts, not this controller cause this controller handles some basic changes for clan, not privilege.

Copy link
Copy Markdown
Collaborator Author

@CapoMK25 CapoMK25 Mar 20, 2026

Choose a reason for hiding this comment

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

I think that this is contradictory, since if I move this entire endpoint away to clanRole.controller.ts instead of this file, I'd be breaking the rules we have set previously where the code changes should stay as minimal as possible and shouldn't introduce anything new unnecessarily. The PUT endpoint on this controller was already on this file originally in almost the exact same format, so I can't justify doing a potentially unnecessary refactor and moving the endpoint to another file when it was originally on this file. Again, that'd break everything that's been discussed previously about keeping the existing endpoints and code as unchanged as they can be.

To quote Niko on the original issue as well:

"Endpoint themselves don't need to change, just that the message no longer automatically creates or modifies or grants roles, but instead starts a vote on whether those happen."

So if the endpoints should stay the exact same as they are right now, moving this endpoint to another file would silently break that promise as the endpoint was originally the almost exact same on this file clan.controller.ts and not clanRole.controller.ts.

Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,12 @@ export class ClanController {
@DetermineClanId()
@HasClanRights([ClanBasicRight.EDIT_CLAN_DATA])
@UniformResponse()
public async update(@Body() body: UpdateClanDto, @LoggedUser() user: User) {
public async update(
@Param('_id') _id: string,
@Body() body: UpdateClanDto,
@LoggedUser() user: User,
) {

if (user.clan_id.toString() !== body._id.toString())
return [
null,
Expand All @@ -214,7 +219,7 @@ export class ClanController {
) {
body.password = this.passwordGenerator.generatePassword('fi');
}
const [, errors] = await this.service.updateOneById(body);
const [, errors] = await this.service.updateOneById(body._id, body);
if (errors) return [null, errors];
}

Expand Down
2 changes: 1 addition & 1 deletion src/clan/clan.module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,6 @@ import { EventEmitterCommonModule } from '../common/service/EventEmitterService/
ClanRoleVotingProcessor,
PasswordGenerator,
],
exports: [ClanService, PlayerCounterFactory, ClanRoleService],
exports: [ClanService, PlayerCounterFactory, ClanRoleService, PasswordGenerator],
})
export class ClanModule {}
Loading