CSC/ECE 517 Fall 2021 - E2140. Create new late policy successfully and fix Bank link: Difference between revisions
(Created page with "This wiki page is for the description of changes made under E2140 OSS assignment for Fall 2021, CSC/ECE 517.") |
No edit summary |
||
Line 1: | Line 1: | ||
This wiki page is for the | |||
==E2140. Create new late policy successfully and fix "Back" link== | |||
This page provides a description of the Expertiza based OSS project, E1240. | |||
__TOC__ | |||
===About Expertiza=== | |||
[http://expertiza.ncsu.edu/ Expertiza] is an open source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages. | |||
===Problem Statement=== | |||
Create new late policy successfull and fix "Back" link. | |||
==Fixes performed=== | |||
* Fix back link on "New late policy" page. | |||
* Fix incorrect flash message on the screen when new late policy is created | |||
* Add controller unit tests for create actions for late_policies_controller | |||
* Add functional test for late policy creation | |||
* Add functional test for back button on new late policy page | |||
* Improve flow of late_policies_controller#edit action for better tests | |||
* Fix late_policies_controller#edit action when it displayed two contradictory flashes | |||
* TODO: SQL stack trace shows up when long string is entered in policy name | |||
* View was checked after a controller test to ensure that the correct message is shown to the user. | |||
* TODO: model was also checked to ensure the database is in a correct state | |||
===About Late Policies Controller=== | |||
This controller can be accessed by editing an assignment, going on "Due Dates" page and clicking on "New late policy". | |||
== Files modified in current project == | |||
1. app/controllers/late_policies_controller.rb <br/> | |||
2. app/views/late_policies/new.html.erb <br/> | |||
3. spec/factories/factories.rb<br/> | |||
==Files added for tests=== | |||
1. spec/controllers/late_policies_controller_spec.rb <br/> | |||
2. spec/features/late_policy_creation_spec.rb <br/> | |||
===About Versions Controller=== | |||
This class manages different versions of reviews. If a reviewer reviews a submission, and after that, the author revises the submission, the next time the reviewer does a review (s)he will create a new version. Sometimes it’s necessary to find the current version of a review; sometimes it’s necessary to find all versions. Similarly, a user may want to delete the current version of a review, or all versions of a review. Pagination of versions helps the user to view a subset of versions at a time. Considering the huge number of versions in the system, it is very useful to have a pagination mechanism and a filtering mechanism which can be applied on the whole set of versions. The idea is to display the versions in an ordered, comprehensible and logical manner. In Expertiza the gem ‘will_paginate’ is used to achieve pagination. | |||
===Current Implementation=== | |||
* There were no tests present in beta branch, so tests for the CRUD operations were added. | |||
* Edit action for late_policies_controller is complex to reason about, with booleans deciding whether there are errors. | |||
* Back button does not lead to the assignment from which the new late policy page was created. | |||
=====Functionality===== | |||
1. | |||
=====Drawbacks and Solutions===== | |||
* '''Problem 1''': Back button was taking user to index of late_policies, and not assignment due dates page | |||
::The back button was performing it's default action of going back to index page of late_policies, but it needs to send user to the assginment page. | |||
Before: <br /> | |||
<pre> | |||
<%= link_to 'Back', :action => 'index' %> | |||
</pre> | |||
After: <br /> | |||
<pre> | |||
<%= link_to 'Back', 'javascript:history.back()' %> | |||
</pre> | |||
* '''Solution''': The link_to function is called with the page previous to current page, so it will go to assginment page. | |||
* Use javascript:history.back() to go back to the previous page | |||
* '''Problem 2''': Creating new policy with blank fields does not give a proper error | |||
::The existing code just shows a blank error, without specifying what has gone wrong. Hence, user will not know what is wrong when they created a new policy. | |||
* '''Solution''': The existing error handling code was changed to show the ActiveRecord::Invalid errors to user. And in case, a different exception occurs, a "something went wrong" message is shown, as this exception shouldn't be visible to users. | |||
Before: | |||
<pre> | |||
rescue StandardError | |||
flash[:error] = "The following error occurred while saving the penalty policy: " | |||
</pre> | |||
After: | |||
<pre> | |||
rescue ActiveRecord::RecordInvalid => exception | |||
flash[:error] = exception.record.errors.full_messages.join("<br />").html_safe | |||
redirect_to action: 'new' | |||
rescue StandardError => exception | |||
# In case it's some error, this error should not be shown to users | |||
flash[:error] = "Something went wrong" | |||
</pre> | |||
* '''Problem 3''': The paginate method can be moved to a helper class. | |||
::VersionsController is not the only component which require to paginate items. There are other components too. For instance, the UsersController has to paginate the list of users. Hence the Paginate method can be moved to a helper class which can be accessed by other components as well. | |||
* '''Solution''': The filtering options has also been enhanced. The current user can now choose as part of the version search filter any user from a list of users if the current user is authorized to see the versions created by that user. | |||
===New Implementation=== | |||
text | |||
<pre> | |||
</pre> | |||
* text | |||
<pre> | |||
</pre> | |||
* text | |||
<pre> | |||
</pre> | |||
===Code improvements=== | |||
* text | |||
<pre> | |||
</pre> | |||
:text | |||
<pre> | |||
</pre> | |||
===Automated Testing using RSPEC=== | |||
TODO: add this section | |||
The current version of expertiza did not have any test for LatePoliciesController. | |||
Using the test driven development(TDD) approach, we have added an exhaustive set of RSPEC tests for LatePoliciesController, to test all the modifications we have done to the code of the controller class. The tests use double and stub features of rspec-rails gem, to fake the log in by different users - Administrator, Instructor, Student etc. The tests can be executed "rpec spec" command as shown below. | |||
<pre> | |||
TODO:: Add execution trace for tests, with time taken, etc | |||
</pre> | |||
===Testing from UI=== | |||
===References=== | |||
#[https://github.com/expertiza/expertiza Expertiza on GitHub] | |||
#[https://github.com/vinay-deshmukh/expertiza GitHub Project Repository Fork] | |||
#[https://github.com/expertiza/expertiza/pull/2087/ Pull request to Expertiza repo] | |||
#[https://github.com/vinay-deshmukh/expertiza/projects/1 GitHub Fork: Project board] | |||
#[https://github.com/vinay-deshmukh/expertiza/issues Issues made while development] | |||
#[https://github.com/vinay-deshmukh/expertiza/pulls Pull requests made to the fork to document work] |
Revision as of 02:08, 19 October 2021
E2140. Create new late policy successfully and fix "Back" link
This page provides a description of the Expertiza based OSS project, E1240.
About Expertiza
Expertiza is an open source project based on Ruby on Rails framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.
Problem Statement
Create new late policy successfull and fix "Back" link.
Fixes performed=
- Fix back link on "New late policy" page.
- Fix incorrect flash message on the screen when new late policy is created
- Add controller unit tests for create actions for late_policies_controller
- Add functional test for late policy creation
- Add functional test for back button on new late policy page
- Improve flow of late_policies_controller#edit action for better tests
- Fix late_policies_controller#edit action when it displayed two contradictory flashes
- TODO: SQL stack trace shows up when long string is entered in policy name
- View was checked after a controller test to ensure that the correct message is shown to the user.
- TODO: model was also checked to ensure the database is in a correct state
About Late Policies Controller
This controller can be accessed by editing an assignment, going on "Due Dates" page and clicking on "New late policy".
Files modified in current project
1. app/controllers/late_policies_controller.rb
2. app/views/late_policies/new.html.erb
3. spec/factories/factories.rb
Files added for tests=
1. spec/controllers/late_policies_controller_spec.rb
2. spec/features/late_policy_creation_spec.rb
About Versions Controller
This class manages different versions of reviews. If a reviewer reviews a submission, and after that, the author revises the submission, the next time the reviewer does a review (s)he will create a new version. Sometimes it’s necessary to find the current version of a review; sometimes it’s necessary to find all versions. Similarly, a user may want to delete the current version of a review, or all versions of a review. Pagination of versions helps the user to view a subset of versions at a time. Considering the huge number of versions in the system, it is very useful to have a pagination mechanism and a filtering mechanism which can be applied on the whole set of versions. The idea is to display the versions in an ordered, comprehensible and logical manner. In Expertiza the gem ‘will_paginate’ is used to achieve pagination.
Current Implementation
- There were no tests present in beta branch, so tests for the CRUD operations were added.
- Edit action for late_policies_controller is complex to reason about, with booleans deciding whether there are errors.
- Back button does not lead to the assignment from which the new late policy page was created.
Functionality
1.
Drawbacks and Solutions
- Problem 1: Back button was taking user to index of late_policies, and not assignment due dates page
- The back button was performing it's default action of going back to index page of late_policies, but it needs to send user to the assginment page.
Before:
<%= link_to 'Back', :action => 'index' %>
After:
<%= link_to 'Back', 'javascript:history.back()' %>
- Solution: The link_to function is called with the page previous to current page, so it will go to assginment page.
- Use javascript:history.back() to go back to the previous page
- Problem 2: Creating new policy with blank fields does not give a proper error
- The existing code just shows a blank error, without specifying what has gone wrong. Hence, user will not know what is wrong when they created a new policy.
- Solution: The existing error handling code was changed to show the ActiveRecord::Invalid errors to user. And in case, a different exception occurs, a "something went wrong" message is shown, as this exception shouldn't be visible to users.
Before:
rescue StandardError flash[:error] = "The following error occurred while saving the penalty policy: "
After:
rescue ActiveRecord::RecordInvalid => exception flash[:error] = exception.record.errors.full_messages.join("<br />").html_safe redirect_to action: 'new' rescue StandardError => exception # In case it's some error, this error should not be shown to users flash[:error] = "Something went wrong"
- Problem 3: The paginate method can be moved to a helper class.
- VersionsController is not the only component which require to paginate items. There are other components too. For instance, the UsersController has to paginate the list of users. Hence the Paginate method can be moved to a helper class which can be accessed by other components as well.
- Solution: The filtering options has also been enhanced. The current user can now choose as part of the version search filter any user from a list of users if the current user is authorized to see the versions created by that user.
New Implementation
text
- text
- text
Code improvements
- text
- text
Automated Testing using RSPEC
TODO: add this section The current version of expertiza did not have any test for LatePoliciesController. Using the test driven development(TDD) approach, we have added an exhaustive set of RSPEC tests for LatePoliciesController, to test all the modifications we have done to the code of the controller class. The tests use double and stub features of rspec-rails gem, to fake the log in by different users - Administrator, Instructor, Student etc. The tests can be executed "rpec spec" command as shown below.
TODO:: Add execution trace for tests, with time taken, etc