Implement functionality for the GET /submissions/:submissionId route and accompanying service file functions

[mays4]

Describe the Feature / Enhancement

Implementing functionality for the GET /assessments/:assessmentId/submissions/:submissionId route which currently does have a test in router test.

This issue continues the work from issue https://github.com/OpenTree-Education/rhizone-lms/issues/515.

Additional Details and Resources

This route requires URL parameter of submissionId (the ID of a assessment submission) and expects no body in the request. The response should contain the following:

Facilitator

itemEnvelope({ "curriculum_assessment": CurriculumAssessment, (with 'questions' and 'answers' and correct answers) "program_assessment": ProgramAssessment, "assessment_submission": AssessmentSubmission (with 'responses') }); Participant

itemEnvelope({ "curriculum_assessment": CurriculumAssessment, (with 'questions' and 'answers' and correct answers (only if graded)) "program_assessment": ProgramAssessment, "assessment_submission": AssessmentSubmission (with 'responses') });

Correct Project Selected

• [X] This issue is linked to the appropriate project

Labels

• [X] This issue has appropriate labels.

[mays4]

EOD , Today worked on adding functions to route and testing in postman Tomorrow continue working in the route and make sure its getting the correct info

[mays4]

EOD today worked on adding functions to the route and try to find the best way to query the required information Tomorrow keep working to solve other issues I have to get the correct data.

[seidior]

Hi Mays!

I know you've been running into some roadblocks trying to write the code that will resolve this issue, and I'd like to break down some of those problems into smaller, more manageable chunks so you have a better idea of what you'll need to complete before your pull request.

A. Write code in the router file to conform to the logic and flow that is set out in the router test file B. Modify the functions that are in the service file (or write new ones) to conform to updated behavior since the opening of the #515 ticket. C. Write tests for those updated service file functions (or tests for the functions assigned to you by the #515 ticket owner). D. Modify the tests in the router test file, if needed, to update them for any changes to parameters or return types of service file functions you've made since beginning this ticket.

Task A: Router File Code

Let's take task A first.

Looking at the code in the #515 branch's router test file for reference, here's what I'm seeing as the logic flow for how your router function should behave:

1. There should be a service file function that you can call that, given a program assessment ID number, returns the corresponding program ID number.
2. There should be a service file function that you can call that, given a principal ID number and a program ID number, returns a string corresponding with that user's permissions within the program ("Participant", "Facilitator", or null).
3. There should be a service file function that you can call that, given a program assessment ID number, returns a ProgramAssessment object corresponding with the matching row in the database.
4. There should be a service file function that you can call that, given the ID number of the curriculum assessment that corresponds with a program assessment as well as boolean values corresponding to whether or not to include (questions and all possible answers) and (correct answers), returns a CurriculumAssessment object corresponding with the matching rows in the database.
5. There should be a service file function that you can call that, given the ID number of an assessment submission and a boolean value corresponding to whether or not to include the submission responses, returns an AssessmentSubmission object corresponding with the matching rows in the database.

As you can see from the test file, every single test for your route follows this logic; it's just that the behaviour of the functions and route differ given different inputs or states. However, I see two issues with this approach:

• Looking at steps 4 and 5, the behavior of step 4 may be dependent on the behavior from step 5, so it's important we move that earlier in the process.
• Looking at step 1 and step 3, it seems like we have two functions that have similar functionality, and we're querying for the same data from the database twice. My suggestion would be to remove step 1 (and delete its corresponding function, getProgramIdByProgramAssessmentId) and consolidate that with step 3, thereby giving this logic:

1. There should be a service file function that you can call that, given a program assessment ID number, returns a ProgramAssessment object corresponding with the matching row in the database. (Since that would give you the program ID number and the curriculum assessment ID number.)
2. There should be a service file function that you can call that, given a principal ID number and a program ID number, returns a string corresponding with that user's permissions within the program ("Participant", "Facilitator", or null).
3. There should be a service file function that you can call that, given the ID number of an assessment submission and a boolean value corresponding to whether or not to include the submission responses, returns an AssessmentSubmission object corresponding with the matching rows in the database.
4. There should be a service file function that you can call that, given the ID number of the curriculum assessment that corresponds with a program assessment as well as boolean values corresponding to whether or not to include (questions and all possible answers) and (correct answers), returns a CurriculumAssessment object corresponding with the matching rows in the database.

Once we have that, let's figure out our function names and signatures.

Looking through our code in competenciesService.ts and programsService.ts, I see the following conventions:

• Functions that start with the word "count" are expected to return the number of matching rows of a query.
• Functions that start with the word "list" are expected to return an array of objects constructed from the matching rows in a database table.
• Functions that start with the words "find" or "get" are expected to return a single object constructed from the matching rows in a database table.
• Functions that start with the word "create" is expected to insert a row into a database table from the parameters of the function call.
• Functions that start with the word "set" or "update" are expected to upsert a row into a database table from the parameters of the function call.

Constructing the functions from that standard:

1. This function should be called findProgramAssessment().
2. This function should be called getPrincipalProgramRole().
3. This function should be called getAssessmentSubmission().
4. This function should be called getCurriculumAssessment().

Back-solving from the function descriptions, this leads us to these function headers:

const findProgramAssessment = (programAssessmentId: number): ProgramAssessment | null => {};
const getPrincipalProgramRole = (principalId: number, programId: number): string | null => {};
const getAssessmentSubmission = (assessmentSubmissionId: number, responsesIncluded?: boolean): AssessmentSubmission | null => {};
const getCurriculumAssessment = (curriculumAssessmentId: number, questionsAndAllAnswersIncluded?: boolean,  questionsAndCorrectAnswersIncluded?: boolean): CurriculumAssessment | null => {};

However, since these need to be async functions, our return types have to be declared as Promises:

const findProgramAssessment = async (programAssessmentId: number): Promise<ProgramAssessment | null> => {};
const getPrincipalProgramRole = async (principalId: number, programId: number): Promise<string | null> => {};
const getAssessmentSubmission = async (assessmentSubmissionId: number, responsesIncluded?: boolean): Promise<AssessmentSubmission | null> => {};
const getCurriculumAssessment = async (curriculumAssessmentId: number, questionsAndAllAnswersIncluded?: boolean, questionsAndCorrectAnswersIncluded?: boolean): Promise<CurriculumAssessment | null> => {};

This should lend itself well for us to begin writing code for our router function:

// get the principal row ID number

// get and parse the program ID number and assessment submission number
// error out if we were passed an invalid program ID number or assessment submission number

// get the program assessment
const programAssessment = await findProgramAssessment(programAssessmentId);

// if program assessment is null, that means there's no matching program assessment. send an error back to the user.
// make sure to call next(new NotFoundError()); return; in order to send the right kind of error back.

// get the principal program role
const programRole = await getPrincipalProgramRole(principalId, programAssessment.program_id);

// if the program role is null, that means the user is not enrolled in the program. send an error back to the user.

// get the assessment submission and responses
const assessmentSubmission = await getAssessmentSubmission(assessmentSubmissionId, true);

// if the assessment submission is null, that means there's no matching assessment submission. send an error back to the user.
// also, if the program role is "Participant" and the principal ID of the AssessmentSubmission doesn't match the logged-in principal ID, we should return an error to the user.

// for this route, we always want to return the questions and all answer options in all cases.
const includeQuestionsAndAllAnswers = true;

// if the program role is facilitator, we should always return the correct answers.
// otherwise, return the correct answers only if the submission is graded
const includeQuestionsAndCorrectAnswers = (programRole === "Facilitator" || assessmentSubmission.assessment_submission_state === "Graded");

// get the curriculum assessment
const curriculumAssessment = await getCurriculumAssessment(programAssessment.assessment_id, includeQuestionsAndAllAnswers, includeQuestionsAndCorrectAnswers);

// let's construct our return value
const assessmentWithSubmission: SavedAssessment = {
  curriculum_assessment: curriculumAssessment,
  program_assessment: programAssessment,
  principal_program_role: programRole,
  submission: assessmentSubmission
};

// let's return that to the user

res.json(itemEnvelope(assessmentWithSubmission));

From there, you should be able to fill in the rest of the missing code above.

Task B: Service Functions

There are two things that I want to highlight with regard to writing the service functions. For convenience sake, let me paste the function declarations here again:

const findProgramAssessment = async (programAssessmentId: number): Promise<ProgramAssessment | null> => {};
const getPrincipalProgramRole = async (principalId: number, programId: number): Promise<string | null> => {};
const getAssessmentSubmission = async (assessmentSubmissionId: number, responsesIncluded?: boolean): Promise<AssessmentSubmission | null> => {};
const getCurriculumAssessment = async (curriculumAssessmentId: number, questionsAndAllAnswersIncluded?: boolean,  questionsAndCorrectAnswersIncluded?: boolean): Promise<CurriculumAssessment | null> => {};

The first is that none of these should be returning an array, nor an object that is exactly the columns and values from the tables from which they are querying. When you query a table, it will naturally want to return an array of objects. So you will need to dereference the array, then transform the returned values into the object type you need to return. You can see some of this going on in the listReflections() function in reflectionsService.ts, for instance, or in constructProgramActivities() in programsService.ts.

The other note is that you should pay close attention in the Knex documentation about which functions need to go in which order when querying the database.

Let's take findProgramAssessment() as an example.

/**
 * Fetches a single program assessment from the database, or null if not found.
 *
 * @param {number} programAssessmentId - The row ID of the program_assessments table for a given program assessment
 * @returns {ProgramAssessment | null} - The ProgramAssessment representation of that program assessment, or null if it was not found.
 */
export const findProgramAssessment = async (programAssessmentId: number): Promise<ProgramAssessment | null> => {
  const matchingProgramAssessmentsRows = await db('program_assessments').select('program_id', 'assessment_id', 'available_after', 'due_date').where('id', programAssessmentId);

  if (matchingProgramAssessmentsRows.length === 0) {
    return null;
  }

  const [programAssessmentRow] = matchingProgramAssessmentsRows;

  const programAssessment: ProgramAssessment = {
    id: programAssessmentId,
    program_id: programAssessmentRow.program_id,
    assessment_id: programAssessmentRow.assessment_id,
    available_after: programAssessmentRow.available_after,
    due_date: programAssessmentRow.due_date
  };

  return programAssessment;
};

As you can see, we handle the case when there's no matching row, and we also construct our return object (which in this case is exactly similar to what we get back from the database, but that won't always be the case) and return that back to the calling function.

Task C: Service Function Tests

When testing the service functions, it's important to keep in mind the calling parameters for mockQuery():

const mockQuery = (sql: string, bindings?: unknown[], response?: unknown) => {}

The first is a string representing the actual SQL query that executes whenever we have a db() function call in our service file functions. The second parameter is an array that represents the bindings, most commonly found when we have a where() in our db() function call. And the third is the response object from our database query. So if I have the following function call in my service file function:

db('program_assessments').select('program_id', 'assessment_id', 'available_after', 'due_date').where('id', programAssessmentId);

Knex is going to turn that into the following SQL query:

select `program_id`, `assessment_id`, `available_after`, `due_date` from `program_assessments` where `id` = ? trx2

Because we have a single question mark, that means we need to bind one variable, so our bindings array should be [programAssessmentId]. If we didn't have any bindings in our query, our bindings array should just be an empty array: [].

The third parameter of mockQuery() is the response object. In nearly every circumstance, this will be an array, since db().select() will return an array, even if there's only one matching row. So before we can call mockQuery(), we need to define what we expect to return from that database query:

const exampleProgramAssessmentId = 4;

const matchingProgramAssessmentsRows = [{
  program_id: 1,
  assessment_id: 3,
  available_after: "2023-03-23 01:23:45",
  due_date: "2023-03-23 01:23:45"
}];

mockQuery(
  'select `program_id`, `assessment_id`, `available_after`, `due_date` from `program_assessments` where `id` = ? trx2',
  [exampleProgramAssessmentId],
  matchingProgramAssessmentsRows
);

Then, you should also specify your return object, which should be explicitly declared as the return type from your function:

const exampleProgramAssessment: ProgramAssessment = {
  id: exampleProgramAssessmentId,
  program_id: matchingProgramAssessmentsRows[0].program_id,
  assessment_id: matchingProgramAssessmentsRows[0].assessment_id,
  available_after: matchingProgramAssessmentsRows[0].available_after,
  due_date: matchingProgramAssessmentsRows[0].due_date
};

expect(await findProgramAssessment(exampleProgramAssessmentId)).toEqual(exampleProgramAssessment);