Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Add points and extended subjects data from warsaw #50

Merged
merged 24 commits into from
May 14, 2022
Merged

Add points and extended subjects data from warsaw #50

merged 24 commits into from
May 14, 2022

Conversation

Anakin100100
Copy link
Contributor

#43

@Anakin100100 Anakin100100 requested a review from a team as a code owner April 21, 2022 11:07
micorix
micorix suggested changes Apr 21, 2022
View reviewed changes
Copy link
Member

@micorix micorix left a comment

Choose a reason for hiding this comment

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

Can you pls include original PDF file and steps to convert it? (We used online Adobe app to extract tables from PDF)?

@micorix
Copy link
Member

micorix commented Apr 24, 2022

@Anakin100100 it would be awesome if we could also parse class name (e.g. 1e) and languages from these strings (" [o] ()

@Anakin100100
Copy link
Contributor Author

@micorix we are ready to merge this. All the requested features have been implemented. Some usage notes:

  • On the frontend please consume the list of subjects that you can filter on from "/subjects" endpoints. This endpoint returns all the subject names that can be filetered on. This has been implemented becuase this list will be frequently changed and having to keep the two lists in sync would be bothersome and bug inducing.
  • The detailed data about a school that you can display on it's page can be accessed at "/institutions/"

@micorix Please be informed that this pull request introduces a lot of technical debt with untested code full of shortcuts and things that need to be refactored so the development of new features will have to be halted for a bit until the codebase is in order again. We are going to merge this only because we are really need to release ASAP.

@micorix
Copy link
Member

micorix commented Apr 27, 2022

Thanks @Anakin100100 . Understood.

  1. Can you mark (comment with TODO or sth) parts in code which require refactoring afterwards?
  2. Please add info about new endpoint and changes in API schema to the API.md file.

@Anakin100100
Copy link
Contributor Author

@micorix can you give an estimate when the first public release could happen? Do you need anything that could be worked on in parallel for now?

micorix
micorix suggested changes May 2, 2022
View reviewed changes
docs/regenerating_the_database.md
@@ -10,6 +10,8 @@ Steps for regenerating the database:
- Enter rails console using rails c -e production in separate terminal
- Delete all Institution objects in the database using Institution.delete_all
- Start the job to create the institution types by running the CreateInstitutionTypesJob.new.perform_now(). Wait for it to finish.
- Start the job to create the Subjects by running the CreateSubjectsJob.new.perform_now(). Wait for it to finish.
Copy link
Member

Choose a reason for hiding this comment

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

Pls add these commands also to regeneration script

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 regeneration script is not present on this branch because it had been branched out before the regeration script branch was merged into master.

Anakin100100
Anakin100100 commented May 2, 2022
View reviewed changes
app/controllers/institutions_controller.rb Show resolved Hide resolved
app/controllers/institutions_controller.rb Outdated Show resolved Hide resolved
app/controllers/institutions_controller.rb Outdated Show resolved Hide resolved
app/controllers/subjects_controller.rb Show resolved Hide resolved
app/services/create_subjects_service.rb Outdated Show resolved Hide resolved
spec/factories/subject_sets.rb Show resolved Hide resolved
spec/factories/subjects.rb Show resolved Hide resolved
spec/models/subject_set_requirements_info_spec.rb Outdated Show resolved Hide resolved
spec/models/subject_set_spec.rb Outdated Show resolved Hide resolved
spec/models/subject_spec.rb Outdated Show resolved Hide resolved
@Anakin100100 Anakin100100 requested a review from micorix May 14, 2022 13:49
@Anakin100100
Copy link
Contributor Author

@micorix Please approve this

@Anakin100100 Anakin100100 merged commit 78b173b into master May 14, 2022
@Anakin100100 Anakin100100 deleted the add_points_and_subjects_data_from_warsaw branch May 14, 2022 19:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@micorix micorix micorix approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Anakin100100 @micorix