Skip to content

Conversation

@ChristopherChudzicki
Copy link
Contributor

@ChristopherChudzicki ChristopherChudzicki commented Aug 21, 2024

What are the relevant tickets?

Closes https://github.com/mitodl/hq/issues/5263

Description (What does it do?)

This PR fixes some fixtures. Currently, the schools.json and departments.json fixtures are not usable—the created_on and updated_on fields are missing. (They were removed in #1253.)

How can this be tested?

  1. On main, run ./manage.py loaddata schools. It will fail
  2. On this branch, run ./manage.py loaddata schools. It will succeed.
  3. Repeat for departments

@ChristopherChudzicki ChristopherChudzicki added the Needs Review An open Pull Request that is ready for review label Aug 21, 2024
@ChristopherChudzicki ChristopherChudzicki marked this pull request as ready for review August 21, 2024 19:49
Comment on lines 9 to 10
# load required fixtures on development by default
python3 manage.py loaddata platforms schools departments offered_by
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The block above was never running. We don't expose NODE_ENV to the web container, nor have we for a while now.

Since run-django-dev.sh is ...for dev ... it seems OK to just always run this line.

That said, also happy to remove this (and restore it to the README).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine, I think. I can't imagine a scenario where you would need to preserve manual edits to any of this data in development, but it might be best to add a quick note to the "Loading fixture files" section of the readme. There's a note about how it's automatically done for you in development, so maybe just elaborate upon that and say that in development data is loaded from fixtures every time, which is what this would do as far as I understand.

@gumaerc gumaerc self-assigned this Aug 22, 2024
Copy link
Contributor

@gumaerc gumaerc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this was my fault! It looks like I accidentally removed these when running dumpdata. I updated the department names in that PR in Django admin, then used dumpdata to regenerate the JSON but apparently missed that it removed these dates. Thanks for fixing that. Anyway, I just have one small suggestion:

Comment on lines 9 to 10
# load required fixtures on development by default
python3 manage.py loaddata platforms schools departments offered_by
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine, I think. I can't imagine a scenario where you would need to preserve manual edits to any of this data in development, but it might be best to add a quick note to the "Loading fixture files" section of the readme. There's a note about how it's automatically done for you in development, so maybe just elaborate upon that and say that in development data is loaded from fixtures every time, which is what this would do as far as I understand.

@ChristopherChudzicki ChristopherChudzicki merged commit 7feecaa into main Aug 22, 2024
@odlbot odlbot mentioned this pull request Aug 22, 2024
4 tasks
@rhysyngsun rhysyngsun deleted the cc/fix-tures branch February 7, 2025 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review An open Pull Request that is ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants