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

Jh/disable module #68

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

Jh/disable module #68

wants to merge 22 commits into from

Conversation

jhurst502
Copy link

No description provided.

@sonarcloud
Copy link

sonarcloud bot commented Sep 21, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jasekiw jasekiw changed the base branch from master to dev September 21, 2021 04:57
src/api/index.ts Show resolved Hide resolved
src/pages/ModuleEditor/ModuleEditorSchema.ts Show resolved Hide resolved
@Cliftonz Cliftonz marked this pull request as draft October 17, 2021 18:05
@jhurst502 jhurst502 marked this pull request as ready for review October 21, 2021 18:28
@Cliftonz Cliftonz self-requested a review October 22, 2021 06:38
@Cliftonz
Copy link
Member

@jhurst502 Can you add a gif or pictures of your front end changes?

@jhurst502
Copy link
Author

image
image
image

@Cliftonz
Copy link
Member

@jhurst502 It looks like you have merge conflicts and code smells

Copy link
Member

@Cliftonz Cliftonz left a comment

Choose a reason for hiding this comment

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

lgtm

src/components/LabListEditor/LabListEditor.tsx Outdated Show resolved Hide resolved
src/components/ModuleCard/ModuleCard.tsx Outdated Show resolved Hide resolved
src/pages/Explore/Explore.tsx Outdated Show resolved Hide resolved
src/pages/ModuleEditor/ModuleEditor.tsx Outdated Show resolved Hide resolved
src/pages/ModuleEditor/ModuleEditor.tsx Outdated Show resolved Hide resolved
src/util/LoadingButton.tsx Outdated Show resolved Hide resolved
src/util/LoadingButton.tsx Outdated Show resolved Hide resolved
src/util/LoadingButton.tsx Outdated Show resolved Hide resolved
@jasekiw
Copy link
Contributor

jasekiw commented Nov 4, 2021

@jhurst502 Request a few changes, let me know if you have any questions

@sonarcloud
Copy link

sonarcloud bot commented Nov 17, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

src/components/LabListEditor/LabListEditor.tsx Outdated Show resolved Hide resolved
src/util/LoadingButton.tsx Outdated Show resolved Hide resolved
@@ -106,6 +107,7 @@ class PublicModule extends Component<PublicModuleProps, MyModuleState> {
loading={this.state.startingModule}
className='btn btn-primary'
onClick={this.hasUserModule() ? undefined : this.startModule}
toolTipText={'This lab is currently disabled'}
Copy link
Contributor

Choose a reason for hiding this comment

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

This tooltip text only shows when the button is disabled but there is no logic here to disable the button

Copy link
Author

Choose a reason for hiding this comment

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

Disabled is set in the LabEnvironment.tsx from the disabled property of the module in the DB.

@@ -117,7 +122,7 @@ export default function ModuleEditor({match: {params: {moduleId}}}: Props) {
<Col className='d-flex justify-content-end align-items-center'>
{editing && Boolean(values.id) && <Button style={{marginRight: '1rem'}} type='button' variant='danger' onClick={onCancel}>Cancel</Button>}
{!editing && <Button type='button' onClick={() => setEditing(true)}>Edit</Button>}
{editing && <LoadingButton loading={isSubmitting} type='submit' label={values.id ? 'Save' : 'Create'}/>}
{editing && <LoadingButton loading={isSubmitting} type='submit' label={values.id ? 'Save' : 'Create'} toolTipText={'This lab is currently disabled'}/>}
Copy link
Contributor

Choose a reason for hiding this comment

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

This tooltip text only shows when the button is disabled but there is no logic here to disable the button. Why would you want to disable this button anyways? Wouldn't we want a person to be able to undisable a mondule by saving it with it dechecked?

Copy link
Author

Choose a reason for hiding this comment

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

The button is disabled if the module is set to disabled by someone authorized to do so. If someone authorized un-disables the module then the button will be un-disabled when the page is refreshed, since disabled is a property of a module stored in the database.

@sonarcloud
Copy link

sonarcloud bot commented Jan 3, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

# Conflicts:
#	src/components/ModuleCard/ModuleCard.tsx
#	src/pages/ModuleEditor/ModuleEditor.tsx
#	src/types/Module.ts
#	yarn.lock
@sonarcloud
Copy link

sonarcloud bot commented Apr 10, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

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.

4 participants