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

Feature: project templates service #516

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DimitriChauvel
Copy link
Member

No description provided.

@@ -16,7 +16,7 @@
class="q-px-none q-py-sm text-caption ellipsis-2-lines"
data-cy="title-container"
>
{{ template.type }}
{{ template.name || template.type }}
Copy link
Member

Choose a reason for hiding this comment

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

template name is a required string, no need || X

Copy link
Member Author

@DimitriChauvel DimitriChauvel Mar 27, 2024

Choose a reason for hiding this comment

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

When HAS_BACKEND is false the data have actually no name field

@@ -90,4 +90,4 @@ onUnmounted(() => {
width: 100%;
}
}
</style>
</style>src/services/TemplatesService
Copy link
Member

Choose a reason for hiding this comment

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

what's this ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done


/**
* @param {string} type - project | component | model
* @returns {Promise<object>}
Copy link
Member

Choose a reason for hiding this comment

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

rework on documentation.
Missing default description, description param are no explain of what is the param
Return have a break line and incomplete description.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

Choose a reason for hiding this comment

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

PROJECT | COMPONENT | MODEL -> , can be 'PROJECT', 'COMPONENT' or 'MODEL'.

* otherwise an error.
*/
export async function getAPITemplatesByType(type) {
return api.get(`/libraries/templates?type=${type.toUpperCase().trim()}`);
Copy link
Member

Choose a reason for hiding this comment

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

no need trim or uppercase

Copy link
Member Author

Choose a reason for hiding this comment

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

I did this to have consistency between the function getTemplatesByType in TemplateManager.js and getAPITemplatesByType but, done.

}

/**
* Get templates have "project" type.
Copy link
Member

Choose a reason for hiding this comment

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

not understandable, rework on sentence

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

export async function getProjectTemplates() {
if (!process.env.HAS_BACKEND) {
const response = await getTemplatesByType('project');
return response;
Copy link
Member

Choose a reason for hiding this comment

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

why do you have a const response, you can directly
return getTemplatesByType(...)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

getTemplatesByType: jest.fn(),
}));

describe('Templates Service', () => {
Copy link
Member

Choose a reason for hiding this comment

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

I think is no like other test file, please check it

Copy link
Member Author

@DimitriChauvel DimitriChauvel Mar 27, 2024

Choose a reason for hiding this comment

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

I dont understand. I just did like in User Service tests file

process.env = OLD_ENV;
});

it('should return a template list not "using" api', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

without calling api

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

describe('Test function: getProjectTemplates', () => {
const OLD_ENV = process.env;

beforeEach(() => {
Copy link
Member

Choose a reason for hiding this comment

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

I think beforeEach and afterAll is useless, you can simplify by adding:
process.env.HAS_BACKEND = true; at the end of your first it

Copy link
Member Author

Choose a reason for hiding this comment

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

it doesn't work.

@DimitriChauvel DimitriChauvel force-pushed the feature/project-templates-service branch 3 times, most recently from 7234e93 to 6818d54 Compare March 27, 2024 15:54
@DimitriChauvel DimitriChauvel force-pushed the feature/project-templates-service branch from 6818d54 to 84d31d7 Compare March 27, 2024 15:58
Copy link

Checking /home/runner/work/leto-modelizer/leto-modelizer/package.json

 @babel/core                              =7.23.6  →  =7.24.3
 @babel/eslint-parser                     =7.23.3  →  =7.24.1
 @babel/preset-env                        =7.23.6  →  =7.24.3
 @badeball/cypress-cucumber-preprocessor  =19.2.0  →  =20.0.3
 @quasar/app-webpack                       =3.9.6  →  =3.12.4
 @vue/test-utils                           =2.4.3  →   =2.4.5
 axios                                     =1.6.2  →   =1.6.8
 core-js                                  =3.34.0  →  =3.36.1
 cypress                                  =13.6.1  →  =13.7.1
 cypress-real-events                      =1.11.0  →  =1.12.0
 eslint                                   =8.56.0  →  =8.57.0
 eslint-plugin-jest                       =27.6.0  →  =27.9.0
 eslint-plugin-jsdoc                      =46.9.1  →  =48.2.1
 eslint-plugin-vue                        =9.19.2  →  =9.24.0
 eslint-webpack-plugin                     =4.0.1  →   =4.1.0
 gherkin-lint                              =4.2.2  →   =4.2.4
 isomorphic-git                           =1.25.1  →  =1.25.6
 monaco-editor                            =0.45.0  →  =0.47.0
 quasar                                   =2.13.0  →  =2.15.1
 v-viewer                                 =3.0.11  →  =3.0.13
 vue                                       =3.3.9  →  =3.4.21
 vue-i18n                                  =9.8.0  →  =9.10.2
 vue-loader                               =17.3.1  →  =17.4.2
 vue-router                                =4.2.5  →   =4.3.0

Run npx npm-check-updates -x leto-modelizer* -u to upgrade package.json

Copy link

sonarcloud bot commented Mar 27, 2024

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