E1911 Refactor Criterion - Revision history

Mkushal: /* The view_completed_question Method */

2019-04-02T04:00:20Z

The view_completed_question Method

← Older revision		Revision as of 04:00, 2 April 2019
Line 309:		Line 309:
	To refactor this method, we need to first refactor the entire response.rb to not append the html string for an entire webpage in a single variable, but rather use appropriate views to render the webpage.		To refactor this method, we need to first refactor the entire response.rb to not append the html string for an entire webpage in a single variable, but rather use appropriate views to render the webpage.

	Refactoring an entire model would be beyond the scope of this project as the scale of the task is that of an entire project by itself.		Refactoring response model would require changes to other question subclasses like scale along with changes to the model itself. We think that it is best to refactor this method when response model is refactored along with other affected subclasses. Also, refactoring an entire model would be beyond the scope of this project as the scale of the task is that of an entire project by itself.

	Thus, this method has been left untouched and still remains in criterion.rb itself.		Thus, this method has been left untouched and still remains in criterion.rb itself.

Mkushal: /* The view_completed_question method */

2019-04-02T03:53:05Z

The view_completed_question method

← Older revision		Revision as of 03:53, 2 April 2019
Line 204:		Line 204:
	===The view_completed_question method===		===The view_completed_question method===

			No changes were made to this method as this method wasn't being called from a view. Instead, it was being called by a model response.rb which was also creating an html string. We think it's best to refactor this method along with response.rb as refactoring this method without refactoring response.rb would make the code messier. More explanation is given in the next section.

	==Proposed Solution==		==Proposed Solution==

Amadhus2: /* Testing */

2019-04-02T00:49:31Z

Testing

← Older revision		Revision as of 00:49, 2 April 2019
Line 348:		Line 348:
	end		end
	</pre>		</pre>

			Now that these methods do not exist, there is no need to keep these test files as there is no html variable being returned or parsed. Therefore, we have removed these redundant test cases.

Amadhus2: /* Testing */

2019-04-02T00:46:54Z

Testing

← Older revision		Revision as of 00:46, 2 April 2019
Line 319:		Line 319:
	it "returns the html " do		it "returns the html " do
	html = criterion.edit(0).to_s		html = criterion.edit(0).to_s
	expect(html).to eq("<tr><td align=\"center\"><a rel=\"nofollow\" data-method=\"delete\" href=\"/questions/1\">Remove</a></td><td><input size=\"6\" value=\"1.0\" name=\"question[1][seq]\" id=\"question_1_seq\" type=\"text\"></td><td><textarea cols=\"50\" rows=\"1\" name=\"question[1][txt]\" id=\"question_1_txt\" placeholder=\"Edit question content here\">test txt</textarea></td><td><input size=\"10\" disabled=\"disabled\" value=\"Criterion\" name=\"question[1][type]\" id=\"question_1_type\" type=\"text\"></td><td><input size=\"2\" value=\"1\" name=\"question[1][weight]\" id=\"question_1_weight\" type=\"text\"></td><td>text area size <input size=\"3\" value=\"\" name=\"question[1][size]\" id=\"question_1_size\" type=\"text\"></td><td> max_label <input size=\"10\" value=\"\" name=\"question[1][max_label]\" id=\"question_1_max_label\" type=\"text\"> min_label <input size=\"12\" value=\"\" name=\"question[1][min_label]\" id=\"question_1_min_label\" type=\"text\"></td></tr>")		expect(html).to eq("<tr><td align=\"center\"><a rel=\"nofollow\" data-method=\"delete\" href=\"/questions/1\">Remove</a></td><td>
			<input size=\"6\" value=\"1.0\" name=\"question[1][seq]\" id=\"question_1_seq\" type=\"text\"></td><td><textarea cols=\"50\" rows=\"1\"
			name=\"question[1][txt]\" id=\"question_1_txt\" placeholder=\"Edit question content here\">test txt</textarea></td><td><input size=\"10\"
			disabled=\"disabled\" value=\"Criterion\" name=\"question[1][type]\" id=\"question_1_type\" type=\"text\"></td><td><input size=\"2\"
			value=\"1\" name=\"question[1][weight]\" id=\"question_1_weight\" type=\"text\"></td><td>text area size <input size=\"3\" value=\"\"
			name=\"question[1][size]\" id=\"question_1_size\" type=\"text\"></td><td> max_label <input size=\"10\" value=\"\" name=\"question[1]
			[max_label]\" id=\"question_1_max_label\" type=\"text\"> min_label <input size=\"12\" value=\"\" name=\"question[1][min_label]\"
			id=\"question_1_min_label\" type=\"text\"></td></tr>")
	end		end
	end		end

Amadhus2: /* Testing */

2019-04-02T00:46:35Z

Testing

← Older revision		Revision as of 00:46, 2 April 2019
Line 313:		Line 313:

	==Testing==		==Testing==

			Since our project only dealt with refactoring existing code, there was no particular need to test any specific components of the application. That being said, there were previous tests that were testing if the html variable returned by those methods was in the expected format or not.
			<pre>
			describe "#edit" do
			it "returns the html " do
			html = criterion.edit(0).to_s
			expect(html).to eq("<tr><td align=\"center\"><a rel=\"nofollow\" data-method=\"delete\" href=\"/questions/1\">Remove</a></td><td><input size=\"6\" value=\"1.0\" name=\"question[1][seq]\" id=\"question_1_seq\" type=\"text\"></td><td><textarea cols=\"50\" rows=\"1\" name=\"question[1][txt]\" id=\"question_1_txt\" placeholder=\"Edit question content here\">test txt</textarea></td><td><input size=\"10\" disabled=\"disabled\" value=\"Criterion\" name=\"question[1][type]\" id=\"question_1_type\" type=\"text\"></td><td><input size=\"2\" value=\"1\" name=\"question[1][weight]\" id=\"question_1_weight\" type=\"text\"></td><td>text area size <input size=\"3\" value=\"\" name=\"question[1][size]\" id=\"question_1_size\" type=\"text\"></td><td> max_label <input size=\"10\" value=\"\" name=\"question[1][max_label]\" id=\"question_1_max_label\" type=\"text\"> min_label <input size=\"12\" value=\"\" name=\"question[1][min_label]\" id=\"question_1_min_label\" type=\"text\"></td></tr>")
			end
			end
			</pre>

			<pre>
			describe "#view_question_text" do
			it "returns the html " do
			html = criterion.view_question_text.to_s
			expect(html).to eq("<TR><TD align=\"left\"> test txt </TD><TD align=\"left\">Criterion</TD><td align=\"center\">1</TD><TD align=\"center\"> () 0 to 5 ()</TD></TR>")
			end
			end
			</pre>

			<pre>
			describe "#complete" do
			it "returns the html " do
			html = criterion.complete(0, nil, 0, 5).to_s
			expect(html).to eq("<div><label for=\"responses_0\">test txt</label></div>")
			end
			end
			</pre>

Amadhus2: /* The view_completed_question Method */

2019-04-02T00:28:43Z

The view_completed_question Method

← Older revision		Revision as of 00:28, 2 April 2019
Line 289:		Line 289:
	code += if question.instance_of? Criterion		code += if question.instance_of? Criterion
	#Answer Tags are enabled only for Criterion questions at the moment.		#Answer Tags are enabled only for Criterion questions at the moment.
	~~<b>~~question.view_completed_question(count, answer, questionnaire_max, tag_prompt_deployments, current_user) \|\| ''~~</b>~~		question.view_completed_question(count, answer, questionnaire_max, tag_prompt_deployments, current_user) \|\| ''
	elsif question.instance_of? Scale		elsif question.instance_of? Scale
	question.view_completed_question(count, answer, questionnaire_max) \|\| ''		question.view_completed_question(count, answer, questionnaire_max) \|\| ''

Amadhus2: /* The view_completed_question Method */

2019-04-02T00:28:04Z

The view_completed_question Method

← Older revision		Revision as of 00:28, 2 April 2019
Line 289:		Line 289:
	code += if question.instance_of? Criterion		code += if question.instance_of? Criterion
	#Answer Tags are enabled only for Criterion questions at the moment.		#Answer Tags are enabled only for Criterion questions at the moment.
	question.view_completed_question(count, answer, questionnaire_max, tag_prompt_deployments, current_user) \|\| ''		<b>question.view_completed_question(count, answer, questionnaire_max, tag_prompt_deployments, current_user) \|\| ''</b>
	elsif question.instance_of? Scale		elsif question.instance_of? Scale
	question.view_completed_question(count, answer, questionnaire_max) \|\| ''		question.view_completed_question(count, answer, questionnaire_max) \|\| ''
Line 311:		Line 311:

	Thus, this method has been left untouched and still remains in criterion.rb itself.		Thus, this method has been left untouched and still remains in criterion.rb itself.

	==Testing==		==Testing==

Amadhus2: /* E1911 Refactoring criterion.rb */

2019-04-02T00:27:26Z

E1911 Refactoring criterion.rb

@@ Line 272: / Line 272: @@
 ===The view_completed_question Method===
-====Affected View Files====
+Instead, this html string is appended to another larger string variable named code in the response model, which builds the entire webpage by iterative appending. The html code for a criterion question being completed is a mere "part" of this entire string, which also includes other things such as the navbar of the page, links to the buttons, other questions, the header and the footer and such.
-'''1.

Sbekkem: /* Affected View Files */

2019-04-01T23:13:16Z

Affected View Files

← Older revision		Revision as of 23:13, 1 April 2019
Line 262:		Line 262:
	<% elsif question.instance_of? Scale %>		<% elsif question.instance_of? Scale %>
	<!--E1911: Replaced the call to criterion.complete method with the newly refactored partial-->		<!--E1911: Replaced the call to criterion.complete method with the newly refactored partial-->
	<%= render partial: 'criterion_complete', :locals => {:question => question, :count => i, :answer => answer, :questionnaire_min => @questionnaire.min_question_score, :questionnaire_max => @questionnaire.max_question_score, :dropdown_or_scale => @dropdown_or_scale} %>		<%= render partial: 'criterion_complete', :locals => {:question => question, :count => i, :answer => answer, :questionnaire_min => @questionnaire.min_question_score, :questionnaire_max => @questionnaire.max_question_score,
			:dropdown_or_scale => @dropdown_or_scale} %>

	<% elsif question.instance_of? Scale %>		<% elsif question.instance_of? Scale %>

Sbekkem: /* Affected View Files */

2019-04-01T23:12:20Z

Affected View Files

← Older revision		Revision as of 23:12, 1 April 2019
Line 268:		Line 268:
	</pre>		</pre>

	'''2. views/~~questionnaires~~/~~_questionnaire.html.erb -''' This partial file is called by new.html.erb when anew question is created. Same logic and changes as above.~~		'''2. views/response/_criterion_complete.html.erb -''' This file has all the html code that was earlier in the complete method of the criterion model. We changed the variables from "self.xxx" to "question.xxx" because self was referring to copy of question object in this context. Apart from this we changed the code to erb format. No more changes were deemed necessary. Please refer to the pull request to refer to the changes in this file as they are pure html and really long to be put in this wiki.
	~~'''3. views/questionnaires/_criterion_edit~~.html.erb -''' This file has all the html code that was earlier in the ~~edit~~ method of the criterion model. We changed the variables from "self.xxx" to "@question.xxx" because self was referring to question object in this context. Apart from this we changed the code to erb format. No more changes were deemed necessary. Please refer to the pull request to refer to the changes in this file as they are pure html and really long to be put in this wiki.

	===The view_completed_question Method===		===The view_completed_question Method===