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

Added Teams Page #134

Closed
wants to merge 37 commits into from
Closed

Added Teams Page #134

wants to merge 37 commits into from

Conversation

yukitya-1811
Copy link
Contributor

@yukitya-1811 yukitya-1811 commented Feb 29, 2024

Description

Added Teams Page for CompSoc, Diode, Piston and IEEE members. Added Core and Faculty models along with profile picture fields.

Fixes # (issue)
#38

Dependencies

List any dependencies that are required for this change.

Type of Change

What types of changes does your code introduce?
Put an x in the boxes that apply

  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Please also list any relevant details for your test configuration. (for bug fix / breaking change)
Put an x in the boxes that apply

  • Test A
  • Test B

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@imApoorva36
Copy link
Contributor

Hey,
The link that you added for navbar large, can you add it to navbar small too?
Since the navbar issue is closed, I believe it'd be better if we kept uniform changes across both navbars .
Thanks :)

Copy link
Member

@nishant-nayak nishant-nayak left a comment

Choose a reason for hiding this comment

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

Left a bunch of comments. Please look into them when you get the time. Thanks!

corpus/accounts/models.py Outdated Show resolved Hide resolved
# TODO: Phase out with GitHub OAuth details
github = models.CharField(
max_length=200, blank=True, null=True, verbose_name="GitHub Username"
)
is_nep = models.BooleanField(default=False, verbose_name="Is NEP Member?")
date_joined = models.DateTimeField(
default=datetime.now(), verbose_name="Date Joined"
verbose_name="Date Joined", default=datetime.now()
Copy link
Member

Choose a reason for hiding this comment

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

Can you use the django.utils.timezone.now() function here instead?



class Core(models.Model):
user = models.OneToOneField(User, null=False, on_delete=models.CASCADE)
Copy link
Member

Choose a reason for hiding this comment

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

Since we're decoupling Core and Faculty, I think this can be a OneToOne to ExecutiveMember instead. Only Executive Members should have the ability to take core positions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey I tried doing that but now I have this issue when I try to makemigrations

It is impossible to add a non-nullable field 'executivemember' to core without specifying a default. This is because the database needs something to populate existing rows.
Please select a fix:
 1) Provide a one-off default now (will be set on all existing rows with a null value for this column)
 2) Quit and manually define a default value in models.py.

I tried deleting all pre-existing model instances from the db and it still throws the error. How can I fix this? It has bugged me a lot in the past too lol.

Further, I cannot open the Core model menu in the django-admin site. It throws this error page at me:

image

Plx help

Copy link
Member

Choose a reason for hiding this comment

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

Do the following:

  • Stop all running containers - docker kill $(docker ps -a -q)
  • Delete the DB volume - docker volume rm postgres_data
  • (Optional) Prune your system - docker system prune
  • Restart all containers - docker compose up --build

Note that this will nuke your existing data in the DB. If you've created temp users and other things, maybe try to dump the data using dumpdata

corpus/accounts/models.py Show resolved Hide resolved
corpus/accounts/models.py Show resolved Hide resolved
corpus/templates/pages/team.html Outdated Show resolved Hide resolved
corpus/accounts/models.py Show resolved Hide resolved
corpus/templates/pages/team.html Outdated Show resolved Hide resolved
corpus/templates/pages/team.html Outdated Show resolved Hide resolved
corpus/templates/pages/team.html Outdated Show resolved Hide resolved
@yukitya-1811 yukitya-1811 linked an issue Mar 12, 2024 that may be closed by this pull request
@yukitya-1811 yukitya-1811 mentioned this pull request Mar 14, 2024
10 tasks
@yukitya-1811 yukitya-1811 mentioned this pull request Mar 15, 2024
12 tasks
@yukitya-1811 yukitya-1811 mentioned this pull request May 8, 2024
4 tasks
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.

Team Page
3 participants