- 
                Notifications
    You must be signed in to change notification settings 
- Fork 1
TD-4893 Delegate activities - view course delegates resulted in 500 error when course admin field applied filters got deleted #2974
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
TD-4893 Delegate activities - view course delegates resulted in 500 error when course admin field applied filters got deleted #2974
Conversation
…rror when course admin field applied filters got deleted
…rror when course admin field applied filters got deleted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The proposed solution does not seem appropriate. This may give error if we use number of admin fields with different option names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per Auldrin's comment. This looks like it will only partially fix the problem. There are 6 custom prompt fields / answers. I suggest we try to a generic approach of catching the error when applying filters, clearing the filters if there is an error and reloading page with cleared filters, if possible.
…/Fixes/TD-4893-Delegateactivities-viewcoursedelegatesresultedin500errorwhencourseadminfieldappliedfiltersgotdeleted
…rror when course admin field applied filters got deleted
…rror when course admin field applied filters got deleted
…rror when course admin field applied filters got deleted
| 
 Please kindly take a look at the latest push | 
| 
 Please kindly take a look at the latest push | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, however some changes need to apply
- To CheckIfFilterisValid(+)
- remove response param if not being used.
- clearFilters param check can be handled by existingFilterString param. Can be removed. (Need to test)
 
- Duplicate filters should be handled within or outside of CheckIfFilterisValid(+)
| 
 Done | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me.
JIRA link
https://hee-tis.atlassian.net/browse/TD-4893
Description
I added Q1Options property to Customisation class
I add Q1Options as a parameter in the GetFilterString method
I added a check to the GetFilterString method to clear the cookievalue where is neccessary
Screenshots
Developer checks
(Leave tasks unticked if they haven't been appropriate for your ticket.)
I have: