Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

DLSV2-581-error-in-sumiting-without-supervisor-and-date-issue - #1434

#1434
Merged
kevwhitt-hee merged 5 commits into
masterTechnologyEnhancedLearning/DLSV2:masterfrom
DLSV2-581-error-in-sumiting-without-supervisorTechnologyEnhancedLearning/DLSV2:DLSV2-581-error-in-sumiting-without-supervisorCopy head branch name to clipboard
Oct 18, 2022
Merged

DLSV2-581-error-in-sumiting-without-supervisor-and-date-issue#1434
kevwhitt-hee merged 5 commits into
masterTechnologyEnhancedLearning/DLSV2:masterfrom
DLSV2-581-error-in-sumiting-without-supervisorTechnologyEnhancedLearning/DLSV2:DLSV2-581-error-in-sumiting-without-supervisorCopy head branch name to clipboard

Conversation

@manishagarwalnhs

@manishagarwalnhs manishagarwalnhs commented Oct 18, 2022

Copy link
Copy Markdown
Contributor

JIRA link

https://hee-dls.atlassian.net/browse/DLSV2-581

Description

fixed issues during submission for date and supervisor id

Screenshots

no screenshots


Developer checks

(Leave tasks unticked if they haven't been appropriate for your ticket.)

I have:

  • Run the formatter and made sure there are no IDE errors (see info on Text Editor settings to avoid whitespace changes)
  • Written tests for the changes (accessibility tests, unit tests for controller, data services, services, view models, etc)
  • Manually tested my work with and without JavaScript
  • Tested any Views or partials created or changed with Wave Chrome plugin and addressed any valid accessibility issues
  • Updated/added documentation in Confluence and/or GitHub Readme. List of documentation links added/changed:
  • Updated my Jira ticket with information about other parts of the system that were touched as part of the MR and have to be sanity tested to ensure nothing’s broken
  • Scanned over my own MR to ensure everything is as expected and it looks right in the browser

@kevwhitt-hee kevwhitt-hee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This fixes issues with enrolling on without selecting a supervisor but I've suggested a couple of tiny tweaks in code.

It would be good to fix the validation error Seema has spotted (we should get rid of the error message that has a typo in it rather than fixing the typo, I think) before releasing.

Almost there!

Comment thread DigitalLearningSolutions.Data/DataServices/CourseDataService.cs
DateTime startedDate = DateTime.Now;
DateTime lastAccessed = startedDate;
string completedDate = "";
dynamic completedDate = "";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This variable should be named completeByDateDynamic rather than completedDate to ensure it has accurate meaning.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

changes has been done

@kevwhitt-hee kevwhitt-hee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These look good. If we could fix the validation message issues spotted by Seema, that would be good.

manishagarwalEviden and others added 2 commits October 18, 2022 16:50
Remove blank line in using statements
@kevwhitt-hee
kevwhitt-hee merged commit c2d85c3 into master Oct 18, 2022
@kevwhitt-hee
kevwhitt-hee deleted the DLSV2-581-error-in-sumiting-without-supervisor branch October 18, 2022 16:00
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.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.