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

Use CharField & Remove "Banner" #166

Merged
merged 5 commits into from
Nov 2, 2023

Conversation

esinx
Copy link
Member

@esinx esinx commented Oct 26, 2023

"Banner" is unnecessary, we will only use "ISSUE" and "NOTICE" but leave space for extensibility later on.

Minor change to use CharField for clarity, frontend would prefer using strings over integers to distinguish the type of announcements.

@esinx esinx requested a review from rm03 October 26, 2023 18:34
@esinx esinx changed the title Use CharField Use CharField & Remove "Banner" Oct 26, 2023
@rm03 rm03 merged commit 7014b93 into feat/announcements Nov 2, 2023
6 of 7 checks passed
@rm03 rm03 deleted the feat/announcements-type-field branch November 2, 2023 17:30
joyliu-q pushed a commit that referenced this pull request Nov 9, 2023
* attempt to setup announcements

* first pass

* add populate command

* oops

* Use CharField & Remove "Banner" (#166)

* fix(announcement): use char instead of integer for announcement_type

* lint: lol I didn't even have flake8 installed

* update Announcement serializer

* unpin black version + lint

* isort

---------

Co-authored-by: Rohan Moniz <[email protected]>

* add filtering on product and end time

* add test cases

* lint

* migration

* address comments

* more validation

---------

Co-authored-by: Eunsoo Shin <[email protected]>
Co-authored-by: Eunsoo Shin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants