Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/more models #7

Closed
wants to merge 16 commits into from
Closed

Feature/more models #7

wants to merge 16 commits into from

Conversation

nmenezes0
Copy link
Contributor

Context

Adding basic models to the Django app for consultations.

Changes proposed in this pull request

Add more models - roughly following the diagram here: https://excalidraw.com/.
Some of the fields will need refining, but I think this is fine as a first-pass (to allow us to progress with other tickets).

Guidance to review

Run the app - does it work ok?
Do the models look sensible based on the diagram?

Link to JIRA ticket

https://technologyprogramme.atlassian.net/browse/CON-24 - doesn't do everything, but part of the Django app set-up.

Things to check

  • I have added any new ENV vars in all deployed environments and updated the .env.example and .env.test files in the repo - N/A

Copy link
Contributor

@duncanjbrown duncanjbrown left a comment

Choose a reason for hiding this comment

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

Sorry for all the red ink 😅 — this looks great. I do think we could leave lots more out for the time being!

README.md Show resolved Hide resolved
consultation_analyser/consultations/models.py Outdated Show resolved Hide resolved
consultation_analyser/consultations/models.py Outdated Show resolved Hide resolved
consultation_analyser/consultations/models.py Show resolved Hide resolved
consultation_analyser/consultations/models.py Outdated Show resolved Hide resolved
consultation_analyser/consultations/models.py Outdated Show resolved Hide resolved
consultation_analyser/consultations/models.py Outdated Show resolved Hide resolved
consultation_analyser/consultations/models.py Show resolved Hide resolved
consultation_analyser/consultations/models.py Outdated Show resolved Hide resolved
consultation_analyser/consultations/models.py Outdated Show resolved Hide resolved
@nmenezes0 nmenezes0 force-pushed the feature/more_models branch from 62a66ef to 47db3b0 Compare March 5, 2024 13:38
Copy link
Contributor

@duncanjbrown duncanjbrown left a comment

Choose a reason for hiding this comment

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

LGTM! Happy to leave order in but in principle I think always better to add stuff like that at the point it's used

@@ -1,19 +1,85 @@
# Generated by Django 5.0.2 on 2024-02-29 14:08
# Generated by Django 5.0.3 on 2024-03-05 16:48
Copy link
Contributor

Choose a reason for hiding this comment

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

Having tried to apply this locally I needed to clear my db first — I think fine to have multiple migrations!

Copy link
Contributor

@duncanjbrown duncanjbrown left a comment

Choose a reason for hiding this comment

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

forgot to send comments earlier!

consultation_analyser/consultations/models.py Outdated Show resolved Hide resolved
consultation_analyser/consultations/models.py Outdated Show resolved Hide resolved
consultation_analyser/consultations/models.py Outdated Show resolved Hide resolved
consultation_analyser/consultations/models.py Show resolved Hide resolved
@nmenezes0 nmenezes0 force-pushed the feature/more_models branch from 3f0e04e to 16c9255 Compare March 7, 2024 14:48
@nmenezes0 nmenezes0 force-pushed the feature/more_models branch from de81d1f to 28b0bee Compare March 8, 2024 14:57
@nmenezes0
Copy link
Contributor Author

Closing as covered in another PR.

@nmenezes0 nmenezes0 closed this Mar 12, 2024
@nmenezes0 nmenezes0 deleted the feature/more_models branch May 30, 2024 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants