addressing non-teaching bug in #250

[rpruim]

See discussion in #250. This adds a computation of whether an item is a non-teaching item if that flag has not been previously set. This removes the dependency on the SectionName column, which we are no longer exporting (since it is redundant information).

[kvlinden]

This bugfix branch didn’t work for me locally. I haven’t run a development server on my home machine for a while, but think that I fired it up properly.

The test that fails is to load a working schedule, write it out and read it right back in. I did this with this existing/old schedule, which has the sectionName field:

?csv=https://kvlinden.github.io/data/full_schedule_2023.csv

I presume that you tried this. ?

keith

[rpruim]

I think that is exactly what I did. I used your link from #250, and actually fetched the CSV file. Read it in, wrote it out, re-exported. I didn't see any issues. Can you describe exactly what problem we should be looking for?

Here's what I see after import/export/re-import locally using the bugfix branch. I sorted by other hours.

[rpruim]

For the record, here's how I'm running locally:

$ pwd
/Users/rpruim/projects/github/course-schedulizer/client-course-schedulizer
[rpruim@ ~/projects/github/course-schedulizer/client-course-schedulizer]
$ git branch
* bugfix/section-name-for-other-duties
  dart-sass
  feature/chairs-meeting-cleanup
  feature/custom-dates
  feature/issue250-csv-export
  feature/issue268-convert-instructional-method
  halfSemesterConflict
  production
[rpruim@ ~/projects/github/course-schedulizer/client-course-schedulizer]
$ npm start

Starting the development server...

  Line 3:10:    'Day' is defined but never used                               @typescript-eslint/no-unused-vars
  Line 49:11:   'meetingStartInternalStr' is assigned a value but never used  @typescript-eslint/no-unused-vars
  Line 51:11:   'meetingEndInternalStr' is assigned a value but never used    @typescript-eslint/no-unused-vars
  Line 56:11:   'roomCapacityStr' is assigned a value but never used          @typescript-eslint/no-unused-vars
  Line 114:13:  'sectionNameStr' is assigned a value but never used           @typescript-eslint/no-unused-vars

Search for the keywords to learn more about each warning.
To ignore, add // eslint-disable-next-line to the line before.

The eslint warnings have been addressed in my other PR.

[kvlinden]

That looks good - there must be something in my local environment that is messing this up.

I'd say push this fix to production expecting that your environment is set up properly.

[rpruim]

I'm not sure I can review my own PR.

[rpruim]

@kvlinden, I've merged into production. Check to see that things are working for you there. If so, we can declare victory for now.

[kvlinden]

The production branch still fails for me. Questions:

• How long do updates to the production branch take to go live?
• Is this the URL you’re using: https://senior-knights.github.io/course-schedulizer/#/?csv=https://kvlinden.github.io/data/full_schedule_2023.csv

Loading this file, then exporting/importing fails for me using Firefox and Chrome on Windows, in both the public browser and the private browser.

[rpruim]

let me do an npm run deploy. I keep forgetting about this step.

Production build being optimized as I type. Should be live in a minute or so.

[rpruim]

The build folder is ready to be deployed.

Find out more about deployment here:

  bit.ly/CRA-deploy

> client-course-schedulizer@0.1.0 deploy
> gh-pages -d build

Published

[rpruim]

works for me from https://senior-knights.github.io/course-schedulizer/#/ now.

[rpruim]

Also from https://senior-knights.github.io/course-schedulizer/#/?csv=https://kvlinden.github.io/data/full_schedule_2023.csv

[kvlinden]

It's better now, but there's still one smaller bug.

The update appears to work for every non-teaching activity entry except the first one. For the fall-schedule2023 example, do the load-export-load cycle again and look at Vic Norman's teaching load fall courses. I see a "--" course there, and note that clicking on this fall courses no longer works. And I confirmed the first-row diagnosis by reordering the non-teaching entries and confirming that whatever is first is messed up.

[rpruim]

Hmm... interesting bug. I wonder if this is related somehow to the logic for checking for an existing section or course.

[rpruim]

Looks like it was roughly what I imagined. There are two else blocks at the end of the import

    // Otherwise, add the new section to the existing course

  // Otherwise, add the new course to the schedule

I put the code to infer the non-teaching status in both blocks. It must be that all the non-teaching items are registered as one "course", so the first one in was in the second else block, which did not have this code. Adding that one line seems to fix the issue @kvlinden is seeing.

I'll create a PR.

Even if this seems to be fixed, we should either leave this open or create a new issue to prompt creating some tests.