freeCodeCamp / classroom

BSD 3-Clause "New" or "Revised" License
144 stars 121 forks source link

Modifying the data type being used for allChallenges in api_processor… #401

Closed ngillux closed 1 year ago

ngillux commented 1 year ago

… from array to object.

Checklist:

Closes #400

GuillermoFloresV commented 1 year ago

If possible, could we have screenshots that show the difference between the old and new approach? Possibly screenshots of the data and what is being processed. Also, is this a fix or possible fix? just so we have an idea on where we are in fixing the problem.

lloydchang commented 1 year ago

@ngillux @GuillermoFloresV

FWIW, I asked OpenAI ChatGPT:

What are the differences between these two lines of JavaScript code? ]['challengeOrder'].map(x => x[0]), ]['challengeOrder'].map(x => Object.values(x)),

ChatGPT replied to me:

The two lines of JavaScript code appear to be very similar, but there is a subtle difference in their mapping functions:

]['challengeOrder'].map(x => x[0]): In this line, ]['challengeOrder'] accesses the property "challengeOrder" of an array (or an object with numerical properties). The map function is then applied to the "challengeOrder" array, and it extracts the first element (index 0) from each element of the array. ]['challengeOrder'].map(x => Object.values(x)): Similar to the first line, ]['challengeOrder'] accesses the property "challengeOrder" of an array (or an object with numerical properties). The map function is then applied, but instead of directly extracting the first element, it extracts all the values of the elements within the "challengeOrder" array using Object.values(x).

So, in summary: The first line extracts the first element from each element in the "challengeOrder" array. The second line extracts all the values of the elements in the "challengeOrder" array and returns them as arrays themselves.

ngillux commented 1 year ago

Hi @lloydchang thanks for your above comment.

Me and @GuillermoFloresV are aware of the difference, we're still figuring out why it was needed / why it was working before and not now.

lloydchang commented 1 year ago

@ngillux @GuillermoFloresV @theGaryLarson

Circling back to https://github.com/freeCodeCamp/classroom/commit/25c41fb79f14f6ae91f8ddedf5ba85c22885d5f4

I asked ChatGPT to diff the before and after from @theGaryLarson's change (detailed below):


ChatGPT replied:

The two code snippets you provided are almost identical. It seems like there might be some duplication in the code as both snippets share the same variable names and processing logic. The only difference between the two snippets is how props.studentData is processed:

First Snippet:

let rawStudentSummary = props.studentData.map(studentJSON => {
  // ... data processing logic using props.studentData as an array ...
});

const data = React.useMemo(
  () => mapData(rawStudentSummary),
  [rawStudentSummary]
);

Second Snippet:

let rawStudentSummary = Object.entries(props.studentData).map(([i]) => {
  // ... data processing logic using props.studentData as an object ...
});

const data = mapData(rawStudentSummary);

In the first snippet, props.studentData is expected to be an array of objects, and the map method is used to process each student's data. In the second snippet, props.studentData is expected to be an object with student data stored as key-value pairs, and Object.entries is used to convert the object into an array of key-value pairs before applying the map method for data processing.

Since the data processing logic is the same in both snippets, it seems that the second snippet is a more correct representation, assuming that the props.studentData is an object.

If there are other differences or specific issues you would like to clarify, please provide more context or details so that I can address them accordingly.


I prompted ChatGPT to diff the before and after from @theGaryLarson's change:

what are the differences between:
  let allCertifications = props.certifications.map(col_course => {
    col_course.selector = row => row[`${col_course.dashedName}`];
    return col_course;
  });

  let numChallengesPerCertification = allCertifications.map(certification => {
    let totalNumChallenges = 0;
    certification.forEach(block => {
      totalNumChallenges += block.allChallenges.length;
    });
    return totalNumChallenges;
  });

  let grandTotalChallenges = 0;
  numChallengesPerCertification.forEach(numChallenges => {
    grandTotalChallenges += numChallenges;
  });

  let rawStudentSummary = props.studentData.map(studentJSON => {
    let studentName = studentJSON.email;
    let superBlocks = Object.values(studentJSON.certifications);
    let blocks = [];
    let blockData = [];
    let completionTimestamps = [];

    superBlocks.forEach(superBlock =>
      blockData.push(Object.values(superBlock))
    );

    let blockName = '';
    blockData.forEach(b =>
      b[0].blocks.forEach(
        obj => ((blockName = Object.keys(obj)[0]), blocks.push(blockName))
      )
    );

    let getCompleted = blockData.flat();

    getCompleted.map(obj =>
      obj.blocks.map(challenges => {
        Object.values(challenges)[0].completedChallenges.map(item => {
          completionTimestamps.push(item.completedDate);
        });
      })
    );

    let rawStudentActivity = {
      recentCompletions: completionTimestamps
    };

    let studentActivity = getStudentActivity(rawStudentActivity);

    let numCompletions = completionTimestamps.length;

    let percentageCompletion = (
      <div>
        <label>
          {numCompletions}/{grandTotalChallenges}{' '}
        </label>
        <meter
          id='progress'
          min='0'
          max={grandTotalChallenges}
          value={numCompletions}
        ></meter>
      </div>
    );

    let studentSummary = {
      name: studentName,
      activity: studentActivity,
      progress: percentageCompletion,
      detail: (
        <a
          // TODO:
          href={
            `/dashboard/v2/details/${props.classroomId}/` + `${studentName}`
          }
        >
          {' '}
          details{' '}
        </a>
      )
    };

    return studentSummary;
  });

  const mapData = function (original_data) {
    let table_data = original_data.map(student => {
      let mapped_student = {
        col1: student.name,
        col2: student.activity,
        col3: student.progress,
        col4: student.detail
      };
      return mapped_student;
    });
    return table_data;
  };

  const data = React.useMemo(
    () => mapData(rawStudentSummary),
    [rawStudentSummary]
  );

  const columns = React.useMemo(
    () => [
      {
        Header: 'Student Name',
        accessor: 'col1', // accessor is the "key" in the data
        width: '20%'
      },
      {
        Header: 'Activity',
        accessor: 'col2',
        width: '10%'
      },
      {
        Header: 'Progress',
        accessor: 'col3',
        width: '20%'
      },
      {
        Header: 'Details',
        accessor: 'col4',
        width: '50%'
      }
    ],
    []
  );
  let allCertifications = props.certifications.map(col_course => {
    col_course.selector = row => row[`${col_course.dashedName}`];
    return col_course;
  });

  let numChallengesPerCertification = allCertifications.map(certification => {
    let totalNumChallenges = 0;
    certification.forEach(block => {
      totalNumChallenges += block.allChallenges.length;
    });
    return totalNumChallenges;
  });

  let grandTotalChallenges = 0;
  numChallengesPerCertification.forEach(numChallenges => {
    grandTotalChallenges += numChallenges;
  });

  let rawStudentSummary = Object.entries(props.studentData).map(([i]) => {
    let studentName = Object.keys(props.studentData[i])[0];
    let completionTimestamps = [];
    let blocks;
    try {
      blocks = Object.keys(props.studentData[i][studentName]['blocks']);
    } catch (e) {
      blocks = [];
    }

    for (let j = 0; j < blocks.length; j++) {
      let studentCompletions =
        props.studentData[i][studentName]['blocks'][blocks[j]][
          'completedChallenges'
        ];

      studentCompletions.forEach(({ completedDate }) => {
        completionTimestamps.push(completedDate);
      });
    }

    let rawStudentActivity = {
      recentCompletions: completionTimestamps
    };
    let studentActivity = getStudentActivity(rawStudentActivity);

    let numCompletions = completionTimestamps.length;
    let percentageCompletion = (
      <div>
        <label>
          {numCompletions}/{grandTotalChallenges}{' '}
        </label>
        <meter
          id='progress'
          min='0'
          max={grandTotalChallenges}
          value={numCompletions}
        ></meter>
      </div>
    );
    let studentSummary = {
      name: studentName,
      activity: studentActivity,
      progress: percentageCompletion,
      detail: (
        <a
          href={
            `/dashboard/v2/details/${props.classroomId}/` + `${studentName}`
          }
        >
          {' '}
          details{' '}
        </a>
      )
    };
    return studentSummary;
  });

  const mapData = function (original_data) {
    let table_data = original_data.map(student => {
      let mapped_student = {
        col1: student.name,
        col2: student.activity,
        col3: student.progress,
        col4: student.detail
      };
      return mapped_student;
    });
    return table_data;
  };

  const data = mapData(rawStudentSummary);

  const columns = [
    {
      Header: 'Student Name',
      accessor: 'col1', // accessor is the "key" in the data
      width: '20%'
    },
    {
      Header: 'Activity',
      accessor: 'col2',
      width: '10%'
    },
    {
      Header: 'Progress',
      accessor: 'col3',
      width: '20%'
    },
    {
      Header: 'Details',
      accessor: 'col4',
      width: '50%'
    }
  ];

  return { data, columns };
}
lloydchang commented 1 year ago

@ngillux @GuillermoFloresV @theGaryLarson

Based on the explanation from ChatGPT:

In the first snippet, props.studentData is expected to be an array of objects, and the map method is used to process each student's data. In the second snippet, props.studentData is expected to be an object with student data stored as key-value pairs, and Object.entries is used to convert the object into an array of key-value pairs before applying the map method for data processing.

As for the question from @ngillux and @GuillermoFloresV :

why it was working before and not now?

My understanding is the difference in object casting.

Previously, it was casted as an array of objects, hence :

x[0]

Now, it's casted as an object, hence:

Object.values(x)
ngillux commented 1 year ago

@lloydchang the response structure of the JSON data changed hence the differences in the two approaches.

lloydchang commented 1 year ago

@ngillux Did the response structure change in a different pull request https://github.com/freeCodeCamp/classroom/pull/366 from @theGaryLarson's https://github.com/freeCodeCamp/classroom/pull/353?

If yes, I believe the fundamental issue is what @theGaryLarson summarized yesterday, "things started to break there because of asynchronous changes in pending PRs."

Meanwhile, I submitted an issue request https://github.com/freeCodeCamp/classroom/issues/403 for @GuillermoFloresV to Enable GitHub's Merge Queue for freeCodeCamp classroom GitHub repository to address the fundamental issue of asynchronous changes in pending PRs.

Thank you!

lloydchang commented 1 year ago

Answering my question to @ngillux earlier:

I believe the response structure changed via https://github.com/freeCodeCamp/freeCodeCamp/pull/50769#pullrequestreview-1502560914 and https://github.com/freeCodeCamp/freeCodeCamp/pull/50955

Specifics are expanded at https://github.com/freeCodeCamp/classroom/issues/410#issuecomment-1657303141