Skip to content

Commit

Permalink
refactor(api): code review
Browse files Browse the repository at this point in the history
  • Loading branch information
er-lim committed Sep 24, 2024
1 parent d4d5214 commit b471e16
Show file tree
Hide file tree
Showing 13 changed files with 32 additions and 30 deletions.
6 changes: 3 additions & 3 deletions api/db/seeds/data/team-acces/build-sco-organizations.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
export function buildScoOrganizations(databaseBuilder) {
_buildCollegeHouseOfTheDragonOrganization(databaseBuilder);
_buildJosephineBaker(databaseBuilder);
_buildScoManagingStudentsNonGarOrganization(databaseBuilder);
_buildScoManagingStudentsNoGarOrganization(databaseBuilder);
}

function _buildCollegeHouseOfTheDragonOrganization(databaseBuilder) {
Expand Down Expand Up @@ -92,13 +92,13 @@ function _buildJosephineBaker(databaseBuilder) {
].forEach(_buildAdminMembership(databaseBuilder));
}

function _buildScoManagingStudentsNonGarOrganization(databaseBuilder) {
function _buildScoManagingStudentsNoGarOrganization(databaseBuilder) {
const organization = databaseBuilder.factory.buildOrganization({
id: SCO_NO_GAR_ORGANIZATION_ID,
type: 'SCO',
name: 'Lycée pas-GAR',
isManagingStudents: true,
email: 'pa[email protected]',
email: 'pas[email protected]',
externalId: ACCESS_SCO_NO_GAR_EXTERNAL_ID,
documentationUrl: 'https://pix.fr/',
provinceCode: '13',
Expand Down
2 changes: 1 addition & 1 deletion api/db/seeds/data/team-acces/constants.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const ACCESS_SCO_BAUDELAIRE_EXTERNAL_ID = 'ACCESS_SCO_BAUDELAIRE';
const ACCESS_SCO_NO_GAR_EXTERNAL_ID = 'ACCESS_SCO_PAGAR';
const ACCESS_SCO_NO_GAR_EXTERNAL_ID = 'ACCESS_SCO_NO_GAR';
const SCO_ORGANIZATION_ID = 2023;
const SCO_NO_GAR_ORGANIZATION_ID = 2024;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const generateOrganizationLearnersUsernameAndTemporaryPassword = async function
});
let organizationLearnerIdentitiesValues = organizationLearnerIdentities.values;

if (!organizationLearnerIdentities.isScoGarIdentityProvider) {
if (!organizationLearnerIdentities.hasScoGarIdentityProvider) {
organizationLearnerIdentitiesValues = await _generateAndUpdateUsernameForOrganizationLearnerIdentities({
organizationLearnerIdentities: organizationLearnerIdentitiesValues,
userReconciliationService,
Expand Down Expand Up @@ -61,7 +61,7 @@ async function _buildOrganizationLearnerIdentities({

return new OrganizationLearnerIdentities({
id: organization.id,
isScoGarIdentityProvider: organization.isGarIdentityProvider,
hasScoGarIdentityProvider: organization.hasGarIdentityProvider,
values: organizationLearnerIdentities,
});
} catch (error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ export class OrganizationLearnerIdentities {
/**
*
* @param {number} id
* @param {boolean} isScoGarIdentityProvider
* @param {boolean} hasScoGarIdentityProvider
* @param {OrganizationLearnerIdentity[]} values
*/
constructor({ id, isScoGarIdentityProvider, values }) {
constructor({ id, hasScoGarIdentityProvider, values }) {
this.id = id;
this.isScoGarIdentityProvider = isScoGarIdentityProvider;
this.hasScoGarIdentityProvider = hasScoGarIdentityProvider;
this.values = values;

this.#assertAllOrganizationLearnerIdentitiesBelongsToOrganization();
Expand Down
5 changes: 3 additions & 2 deletions api/src/organizational-entities/domain/models/Organization.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { NON_OIDC_IDENTITY_PROVIDERS } from '../../../identity-access-management/domain/constants/identity-providers.js';
import { Tag } from './Tag.js';

const types = {
Expand Down Expand Up @@ -66,8 +67,8 @@ class Organization {
return this.archivedAt !== null;
}

get isGarIdentityProvider() {
return this.isScoAndManagingStudents && this.identityProviderForCampaigns === 'GAR';
get hasGarIdentityProvider() {
return this.isScoAndManagingStudents && this.identityProviderForCampaigns === NON_OIDC_IDENTITY_PROVIDERS.GAR.code;
}

get isPro() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ describe('Unit | Identity Access Management | Domain | Model | OrganizationIdent
// when
const organizationIdentity = new OrganizationLearnerIdentities({
id: organizationId,
isScoGarIdentityProvider: true,
hasScoGarIdentityProvider: true,
values,
});

// then
expect(organizationIdentity.id).to.eq(organizationId);
expect(organizationIdentity.isScoGarIdentityProvider).to.be.true;
expect(organizationIdentity.hasScoGarIdentityProvider).to.be.true;
expect(organizationIdentity.values).to.deep.equal(values);
});

Expand All @@ -58,6 +58,7 @@ describe('Unit | Identity Access Management | Domain | Model | OrganizationIdent
() =>
new OrganizationLearnerIdentities({
id: organizationId,
hasScoGarIdentityProvider: true,
values,
}),
)();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('Unit | Organizational Entities | Domain | Model | Organization', funct
});
});

describe('get#isGarIdentityProvider', function () {
describe('get#hasGarIdentityProvider', function () {
context(
"when organization has an 'identityProviderForCampaigns' field with Gar value and isScoAndManagingStudents",
function () {
Expand All @@ -72,7 +72,7 @@ describe('Unit | Organizational Entities | Domain | Model | Organization', funct
});

// when & then
expect(organization.isGarIdentityProvider).to.be.true;
expect(organization.hasGarIdentityProvider).to.be.true;
});
},
);
Expand All @@ -86,7 +86,7 @@ describe('Unit | Organizational Entities | Domain | Model | Organization', funct
});

// when & then
expect(organization.isGarIdentityProvider).to.be.false;
expect(organization.hasGarIdentityProvider).to.be.false;
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import ActionBar from '../ui/action-bar';
{{t "pages.sco-organization-participants.action-bar.information" count=@count}}
</:information>
<:actions>
{{#if @isGarIdentityProvider}}
{{#if @hasGarIdentityProvider}}
<PixButton @triggerAction={{@openResetPasswordModal}}>
{{t "pages.sco-organization-participants.action-bar.reset-password-button"}}
</PixButton>
Expand Down
2 changes: 1 addition & 1 deletion orga/app/components/sco-organization-participant/list.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
@count={{selectedStudents.length}}
@openGenerateUsernamePasswordModal={{fn this.openGenerateUsernamePasswordModal selectedStudents}}
@openResetPasswordModal={{fn this.openResetPasswordModal selectedStudents}}
@isGarIdentityProvider={{this.isGarIdentityProvider}}
@hasGarIdentityProvider={{this.hasGarIdentityProvider}}
/>
<ScoOrganizationParticipant::ResetPasswordModal
@showModal={{this.showResetPasswordModal}}
Expand Down
4 changes: 2 additions & 2 deletions orga/app/components/sco-organization-participant/list.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ export default class ScoList extends Component {
return Boolean(this.args.students.length);
}

get isGarIdentityProvider() {
return this.currentUser.organization.isGarIdentityProvider;
get hasGarIdentityProvider() {
return this.currentUser.organization.hasGarIdentityProvider;
}

@action
Expand Down
2 changes: 1 addition & 1 deletion orga/app/models/organization.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export default class Organization extends Model {
@hasMany('group', { async: true, inverse: null }) groups;
@hasMany('division', { async: true, inverse: null }) divisions;

get isGarIdentityProvider() {
get hasGarIdentityProvider() {
return this.isScoAndManagingStudents && this.identityProviderForCampaigns === 'GAR';
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ module('Integration | Component | ScoOrganizationParticipant::ListActionBar', fu
});
});

module('When isGarIdentityProvider arg is at true', function () {
const isGarIdentityProvider = true;
module('When hasGarIdentityProvider arg is at true', function () {
const hasGarIdentityProvider = true;

test('displays reset passwords and it works correctly', async function (assert) {
// given
Expand All @@ -46,7 +46,7 @@ module('Integration | Component | ScoOrganizationParticipant::ListActionBar', fu
<template>
<ListActionBar
@count={{count}}
@isGarIdentityProvider={{isGarIdentityProvider}}
@hasGarIdentityProvider={{hasGarIdentityProvider}}
@openGenerateUsernamePasswordModal={{openGenerateUsernamePasswordModal}}
@openResetPasswordModal={{openResetPasswordModal}}
/>
Expand All @@ -65,8 +65,8 @@ module('Integration | Component | ScoOrganizationParticipant::ListActionBar', fu
});
});

module('When isGarIdentityProvider arg is at false', function () {
const isGarIdentityProvider = false;
module('When hasGarIdentityProvider arg is at false', function () {
const hasGarIdentityProvider = false;

test('displays generates username password button and it works correctly', async function (assert) {
// given
Expand All @@ -77,7 +77,7 @@ module('Integration | Component | ScoOrganizationParticipant::ListActionBar', fu
<template>
<ListActionBar
@count={{count}}
@isGarIdentityProvider={{isGarIdentityProvider}}
@hasGarIdentityProvider={{hasGarIdentityProvider}}
@openGenerateUsernamePasswordModal={{openGenerateUsernamePasswordModal}}
@openResetPasswordModal={{openResetPasswordModal}}
/>
Expand Down
6 changes: 3 additions & 3 deletions orga/tests/unit/models/organization-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ module('Unit | Model | organization', function (hooks) {
assert.false(organization.isScoAndManagingStudents);
});
});
module('#isGarIdentityProvider', function () {
module('#hasGarIdentityProvider', function () {
test("it returns true when organization isScoAndManagingStudents and identityProviderForCampaigns 'GAR'", function (assert) {
// given
const store = this.owner.lookup('service:store');
Expand All @@ -46,7 +46,7 @@ module('Unit | Model | organization', function (hooks) {
});

// then
assert.true(organization.isGarIdentityProvider);
assert.true(organization.hasGarIdentityProvider);
});

test("it returns false when organization does not have isScoAndManagingStudents or identityProviderForCampaigns 'GAR'", function (assert) {
Expand All @@ -61,7 +61,7 @@ module('Unit | Model | organization', function (hooks) {
});

// then
assert.false(organization.isGarIdentityProvider);
assert.false(organization.hasGarIdentityProvider);
});
});
});

0 comments on commit b471e16

Please sign in to comment.