Issue #CO-229 #CO-230 #CO-316 fix: {Reraising PR on release-6.0.0} #303

rekhaDevs · 2023-04-05T10:22:07Z

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Type of change

Please choose appropriate options.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes in the below checkboxes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Ran Test A
Ran Test B

Test Configuration:

Software versions: Nodejs 8.11.2
Hardware versions:

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

abhay-samagra · 2023-05-18T09:31:29Z

@vinukumar-vs - Please have a look at the PR.

vinukumar-vs · 2023-06-29T14:52:55Z

src/service/print/getdocxdata.js

@@ -711,8 +713,8 @@ function displayNumber(data) {
          type: WidthType.DXA,
        },
        margins: {
-          top: convertInchesToTwip(0.0693701),
-          bottom: convertInchesToTwip(0.0693701),
+          top: convertInchesToTwip(0.0093701),


This is not the right way of fixing the margins. It should be common across all sides. Any specific reason why the margins are different for the document.

While printing paper having MCQ questions, the line height between line 1 of options and line 2 of options was a bit higher. It was done to reduce the line height space to consume less physical paper.

Not convinced.. Can you share the snapshot of the UI.

Without altering Margin:

With altering Margin:

vinukumar-vs · 2023-06-29T14:54:22Z

src/service/print/docxHelper.js

@@ -270,20 +275,20 @@ async function renderMCQ(question, questionCounter, marks) {
      typeof questionOptions[0][0] === "object"
    ) {
      if (questionOptions[0][0].text) {
-        questionOpt.push(["I.", questionOptions[0][0].text[1]]);
+        questionOpt.push([`${getLanguageKey(printLanguage, 'i')+")"}`, questionOptions[0][0].text[1]]);


@vaivk369 suggest the better approach of pushing the question to the array.

vinukumar-vs · 2023-06-29T14:59:04Z

src/service/print/docxHelper.js

 const ProgramServiceHelper = require("../../helpers/programHelper");
 const programServiceHelper = new ProgramServiceHelper();
+const optionLabelling = {
+  english: {i: 'I', ii: 'II', iii: 'III', iv: 'IV'},


Why are the using the keys as i, ii, iii, iv, why can't it be just an array of strings?

optionLabelling = {
"en": ["I", "II", "III" ......],
"hi": ["क", "ख", "ग" ......]
}

This should be configuration driven. should not hardcode here. Later if we want another language, it should be config change. Code should work.

Yes will be implementing this.

Shall I resolve this conversation, as this one is implemented?

vinukumar-vs · 2023-06-29T14:59:48Z

src/service/print/docxHelper.js

@@ -297,20 +302,20 @@ async function renderMCQ(question, questionCounter, marks) {
      typeof questionOptions[1][0] === "object"
    ) {
      if (questionOptions[1][0].text) {
-        questionOpt.push(["II.", questionOptions[1][0].text[1]]);
+        questionOpt.push([`${getLanguageKey(printLanguage, 'ii')+")"}`, questionOptions[1][0].text[1]]);


Why should I use the same key "II" for all the languages.

Cause there must be default options labeling in case of undefined language.

Shall I resolve this conversation?

vinukumar-vs · 2023-06-29T16:12:35Z

src/service/print/docxHelper.js

        imageProperties.push({
          width: questionOptions[0][0].width,
          height: questionOptions[0][0].height,
        });
      }
    } else {
-      questionOpt.push(["I.", questionOptions[0][0]]);
+      questionOpt.push([`${getLanguageKey(printLanguage, 'i')+")"}`, questionOptions[0][0]]);


Why can't you move the entire this logic into different function as pass the index to it
for loop
setOption(index, questionOptions)

function setOption(index, questionOptions) if (questionOptions[index] !== undefined) { if ( questionOptions[index][0] !== undefined && typeof questionOptions[index][0] === "object" ) { if (questionOptions[index][0].text) { questionOpt.push([`${getLanguageKey(printLanguage, index)+")"}`, questionOptions[index][0].text[1]]); imageProperties.push({ width: 0, height: 0, }); } else { questionOpt.push([`${getLanguageKey(printLanguage, index+1)+")"}`, questionOptions[index][0].image]); imageProperties.push({ width: questionOptions[index][0].width, height: questionOptions[index][0].height, }); } } else { questionOpt.push([`${getLanguageKey(printLanguage, index+1)+")"}`, questionOptions[index][0]]); imageProperties.push({ width: 0, height: 0, }); } }

This way if there are only 2 option or more than 4 also the code will works.

This is done @vinukumar-vs

Shall I resolve this conversation, as this is implemented too?

vinukumar-vs · 2023-06-29T16:15:23Z

src/service/print/lang/english.json

@@ -0,0 +1,9 @@
+{
+    "studentName" : "Student Name",


These files should be taken from blob rather than adding to the code repo. Any new language should not require any code code, it will be configuration driven.

Where must i save these language files and how we will be using them. If there is any reference, please share.

@vaivk369 , please help me with this if you can.

vinukumar-vs · 2023-07-07T04:26:42Z

src/service/print/docxHelper.js

@@ -394,6 +297,36 @@ async function renderMCQ(question, questionCounter, marks) {
  return data;
 }

+function setOptions(questionOptions, questionOpt, imageProperties, printLanguage, i) {


Hope you enjoyed writing the code.. looks fine.

@vaivk369 @shubhambansaltarento

yes i did, Thank you for guidance.

src/service/print/docxHelper.js

vinukumar-vs · 2023-07-07T04:27:45Z

src/service/print/getdocxdata.js

@@ -711,8 +713,8 @@ function displayNumber(data) {
          type: WidthType.DXA,
        },
        margins: {
-          top: convertInchesToTwip(0.0693701),
-          bottom: convertInchesToTwip(0.0693701),
+          top: convertInchesToTwip(0.0093701),


Not convinced.. Can you share the snapshot of the UI.

… into co-316

co-316: changes

8656429

rekhaDevs changed the title ~~co-316: changes~~ Issue #CO-229 #CO-230 #CO-316 fix: {Reraising PR on release-6.0.0} Apr 5, 2023

vinukumar-vs approved these changes May 18, 2023

View reviewed changes

Updated branch from Release-6.0.0

0b37f67

vinukumar-vs suggested changes Jun 29, 2023

View reviewed changes

rekhaDevs closed this Jul 5, 2023

rekhaDevs force-pushed the co-316 branch from c1c8c75 to d32d8b3 Compare July 5, 2023 13:44

rekhaDevs reopened this Jul 5, 2023

rekhaDevs force-pushed the co-316 branch from c1c8c75 to 0b37f67 Compare July 5, 2023 13:48

code optimization

9c4622e

vinukumar-vs reviewed Jul 7, 2023

View reviewed changes

Rekha added 4 commits July 12, 2023 16:16

Merge branch 'release-6.0.0' of github.com:Sunbird-Ed/program-service…

ba0fe42

… into co-316

generated data optimisation

f180ae8

added reverted code

4774ce6

approved and unapproved question

9220d28

vinukumar-vs changed the base branch from release-6.0.0 to release-6.0.1 August 24, 2023 04:49

vinukumar-vs approved these changes Aug 24, 2023

View reviewed changes

vinukumar-vs merged commit 905e421 into Sunbird-coKreat:release-6.0.1 Aug 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #CO-229 #CO-230 #CO-316 fix: {Reraising PR on release-6.0.0} #303

Issue #CO-229 #CO-230 #CO-316 fix: {Reraising PR on release-6.0.0} #303

rekhaDevs commented Apr 5, 2023

abhay-samagra commented May 18, 2023

vinukumar-vs Jun 29, 2023

rekhaDevs Jul 4, 2023

vinukumar-vs Jul 7, 2023

rekhaDevs Jul 13, 2023

vinukumar-vs Jun 29, 2023

vinukumar-vs Jun 29, 2023

rekhaDevs Jul 4, 2023

rekhaDevs Jul 13, 2023

vinukumar-vs Jun 29, 2023

rekhaDevs Jul 4, 2023

rekhaDevs Jul 13, 2023

vinukumar-vs Jun 29, 2023

vinukumar-vs Jun 29, 2023

rekhaDevs Jul 6, 2023

rekhaDevs Jul 13, 2023

vinukumar-vs Jun 29, 2023

rekhaDevs Jul 5, 2023

rekhaDevs Jul 5, 2023

vinukumar-vs Jul 7, 2023

rekhaDevs Jul 13, 2023

vinukumar-vs Jul 7, 2023

Issue #CO-229 #CO-230 #CO-316 fix: {Reraising PR on release-6.0.0} #303

Issue #CO-229 #CO-230 #CO-316 fix: {Reraising PR on release-6.0.0} #303

Conversation

rekhaDevs commented Apr 5, 2023

Type of change

How Has This Been Tested?

Checklist:

abhay-samagra commented May 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment