search

freeCodeCamp / classroom

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

The classes dashboard page is not processing the student JSON data correctly #386

Closed utsab closed 1 year ago

utsab commented 1 year ago

Looking forward for reporting a security issue: Please report security issues by following our security policy: https://contribute.freecodecamp.org/#/security

Describe the bug In a recent pull request, a merge conflict occurred in the student-data-processing code (util/processDashboardData.js). This merge conflict was resolved incorrectly. As a result, the dashboard table no longer parses the student json correctly and the wrong values show up in the table.

To Reproduce Steps to reproduce the behavior:

  1. Click on the "Classes tab"
  2. Create a Class
  3. View the Class
  4. Note that the The Student Name column of the dashboard table is showing the text "email" for all students.

Expected behavior We should see valid email addresses in the column. These email addresses should match the student emails defined in the mock data.

Screenshots If applicable, add screenshots to help explain your problem.

Screen Shot 2023-07-23 at 12 03 30 PM
utsab commented 1 year ago

Assigning to @ngillux

lloydchang commented 1 year ago

Cross-posting https://codedayorg.slack.com/archives/C05E350DQH4/p1690332408205079?thread_ts=1689637794.458589

I understand that @Utsab assigned https://github.com/freeCodeCamp/classroom/issues/386 to @ngillux

With that said, @theGaryLarson is looking at https://github.com/freeCodeCamp/classroom/issues/386 just in case the issue originated from a merge conflict resolution by @theGaryLarson for https://github.com/freeCodeCamp/classroom/pull/353 then merged by @GuillermoFloresV

I don't know what Git merge strategy freeCodeCamp Classroom is using by default, but the issue could have also been caused by GitHub / Git instead of a human.

lloydchang commented 1 year ago

Conversation from CodeDay Labs Slack https://codedayorg.slack.com/archives/C05E350DQH4/p1690332562033589?thread_ts=1689637794.458589

@theGaryLarson wrote:

@lloydchang regarding https://github.com/freeCodeCamp/classroom/issues/386 I went to the previous commit before and I am still getting the same error when i view the page. Which I wasn't before even after my changes. I'm not sure how to review what I did wrong so I can learn from it. It was strange to me there was a merge conflict in the first place. And it is confusing that the merge conflict is reported in util/processDashboardData.js. That is a new file. The merge conflict that showed up for me was in components/dastable_v2.js where I had moved the code linked here to its own file.

image

No matter the commit I go to I am getting this error now when trying to view the details page of a classroom.

@lloydchang replied:

@theGaryLarson You could try git bisect https://www.metaltoad.com/blog/beginners-guide-git-bisect-process-elimination

@GuillermoFloresV replied:

Hi sorry, this seemed to be an issue regarding syncing. The current running theory is : When you look at the initial commit on your PR, @ngillux's changes in this PR are not present, this shows the disconnect between main and the code we expected to be there later on. Because of this, there was a merge conflict inside of the dashtable V2 component since I believe her PR was merged first (I'm currently on mobile so I can't confirm this 100% but I seem to remember this being true.) The conflicts were fixed by accepting local changes and then in our final changes, we simply pushed "old code" to main, effectively causing the issue in #386 It's assumed this conflict was resolved by accepting local changes, rather than the new ones authored by Natalia. If this theory is right, it was an oversight on my end during the review of the PR after the merge conflicts were fixed.

@theGaryLarson replied:

Thanks Guillermo that helps me rest easier. I was worried I was breaking the code!