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.

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

Jump to bottom

[#9584] Change error message for Instructor Course Details Ajax failure #9591

Merged
merged 7 commits into from Apr 9, 2019
Merged

[#9584] Change error message for Instructor Course Details Ajax failure #9591

merged 7 commits into from Apr 9, 2019

Conversation

a-deeb
Copy link
Contributor

@a-deeb a-deeb commented Mar 20, 2019 •
edited

Fixes #9584

PR Checklist

Ensure that you have:

  • [ x] Read and understood our PR guideline: https://github.com/TEAMMATES/teammates/blob/master/docs/process.md#step-4-submit-a-pr
    • [ x] Added the issue number to the "Fixes" keyword above
    • [x ] Titled the PR as specified in the abovementioned document
  • [x ] Made your changes on a branch other than master and release
  • [x ] Gone through all the changes in this PR and ensured that:
    • [ x] They addressed one (and only one) issue
    • [ x] No unintended changes were made
  • [x ] Run and passed static analysis: ./gradlew lint and npm run lint
  • [x ] Added/updated tests, if changes in functionality were involved
  • [x ] Added/updated documentation to public APIs (classes, methods, variables), if applicable

Outline of Solution

Changed the error message as Part of #9584

Remove mention of modal in Ajax error message

@teammates-bot
Copy link

Hi @a-deeb, these parts of your pull request do not appear to follow our contributing guidelines:

  1. PR Title
    • Issue Reference (#<issue-number>) missing.
  2. PR Description

@a-deeb a-deeb changed the title Update instructor-course-details-page.component.html Update instructor-course-details-page.component.html #9584 Mar 20, 2019
@teammates-bot
Copy link

Hi @a-deeb, these parts of your pull request do not appear to follow our contributing guidelines:

  1. PR Title
  2. PR Description

1 similar comment
@teammates-bot
Copy link

Hi @a-deeb, these parts of your pull request do not appear to follow our contributing guidelines:

  1. PR Title
  2. PR Description

@teammates-bot
Copy link

Hi @a-deeb, these parts of your pull request do not appear to follow our contributing guidelines:

  1. PR Title

@a-deeb a-deeb changed the title Update instructor-course-details-page.component.html #9584 [#9584] Update instructor-course-details-page.component.html Mar 20, 2019
@a-deeb a-deeb changed the title [#9584] Update instructor-course-details-page.component.html [#9584] Update instructor-course-details-page.component.html (#9591) Mar 20, 2019
@a-deeb a-deeb changed the title [#9584] Update instructor-course-details-page.component.html (#9591) [#9584] Update instructor-course-details-page.component.html Mar 20, 2019
AyushChatto
AyushChatto requested changes Mar 20, 2019
View reviewed changes
Copy link
Contributor

@AyushChatto AyushChatto left a comment

Choose a reason for hiding this comment

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

May also want to tell them to try reloading. Also there is a pending PR for the snapshot test for this component that might get merged before this is done, so you may want to hold out for that and update the test accordingly.

@AyushChatto AyushChatto added the s.Ongoing The PR is being worked on by the author(s) label Mar 20, 2019
AyushChatto
AyushChatto approved these changes Mar 31, 2019
View reviewed changes
Copy link
Contributor

@AyushChatto AyushChatto left a comment

Choose a reason for hiding this comment

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

Just fix the minor issue. Rest LGTM :)

...ages-instructor/instructor-course-details-page/instructor-course-details-page.component.html Outdated Show resolved Hide resolved
@AyushChatto AyushChatto added s.FinalReview The PR is ready for final review and removed s.Ongoing The PR is being worked on by the author(s) labels Mar 31, 2019
wkurniawan07
wkurniawan07 requested changes Apr 3, 2019
View reviewed changes
...ages-instructor/instructor-course-details-page/instructor-course-details-page.component.html Outdated
@@ -135,7 +135,7 @@ <h4 class="modal-title">Confirm Deletion</h4>
<div class="modal-body">
<tm-ajax-preload *ngIf="loading"></tm-ajax-preload>
<div [innerHTML]="convertToHtmlTable(courseStudentListAsCsv)"></div>
<b>{{ isAjaxSuccess ? '' : 'Failed to load. Please reopen modal.' }}</b>
<b>{{ isAjaxSuccess ? '' : 'There was an error loading this file. Please try again.' }}</b>
Copy link
Member

Choose a reason for hiding this comment

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

There is no "this file" involved here. "Data" is more appropriate.

wkurniawan07
wkurniawan07 approved these changes Apr 7, 2019
View reviewed changes
Copy link
Member

@wkurniawan07 wkurniawan07 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for contributing!

@wkurniawan07 wkurniawan07 added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.FinalReview The PR is ready for final review labels Apr 7, 2019
@wkurniawan07 wkurniawan07 changed the title [#9584] Update instructor-course-details-page.component.html [#9584] Change error message for Instructor Course Details Ajax failure Apr 8, 2019
@RonakLakhotia RonakLakhotia merged commit 0861182 into TEAMMATES:master Apr 9, 2019
@wkurniawan07 wkurniawan07 added this to the V7.0.0-beta.0 milestone Apr 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wkurniawan07 wkurniawan07 wkurniawan07 approved these changes

@AyushChatto AyushChatto AyushChatto approved these changes

Assignees
No one assigned
Labels
s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Milestone
V7.0.0-beta.0
Development

Successfully merging this pull request may close these issues.

Change error message for Instructor Course Details Ajax failure
5 participants
@a-deeb @teammates-bot @wkurniawan07 @AyushChatto @RonakLakhotia