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

Remove admin_comment from company #3643

Closed
wants to merge 101 commits into from

Conversation

juniwbjerde
Copy link
Member

@juniwbjerde juniwbjerde commented Sep 25, 2024

Remove the field admin_comment from the company model. It was never really used...

I also wrote a custom data migration to make a normal comment on the company with the admin_comment (in case any of the admin_comments were actually useful and shouldn't be totally deleted

Corresponding frontend changes: webkom/lego-webapp#5001

Resolves ABA-1052

itsisak and others added 2 commits May 22, 2024 13:03
Bumps [twisted](https://github.com/twisted/twisted) from 22.10.0 to 24.7.0.
- [Release notes](https://github.com/twisted/twisted/releases)
- [Changelog](https://github.com/twisted/twisted/blob/trunk/NEWS.rst)
- [Commits](twisted/twisted@twisted-22.10.0...twisted-24.7.0)

---
updated-dependencies:
- dependency-name: twisted
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Copy link

linear bot commented Sep 25, 2024

Copy link
Member

@ivarnakken ivarnakken left a comment

Choose a reason for hiding this comment

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

Should we totally remove it? I thought we only wanted to remove it from the bdb table since it took up space. I don't think we need to remove it as a feature, but instead move it to a better place (like itDAGENE's admin pages haha)

@ivarnakken ivarnakken added discussion Pull requests or issues that need discussion, e.g. regarding controversial/big changes review-needed Pull requests that need review small-fix Pull requests that fix something small labels Sep 25, 2024
@juniwbjerde
Copy link
Member Author

juniwbjerde commented Sep 26, 2024

@ivarnakken I talked to Falk about it, and his impression after talking to Bedkom was that they didn't use it. I don't see why we would need both the admin_comment field as well as normal comments on companies. It's just a bit messy in my opinion, and there's nothing you would write in the admin comment that wouldn't fit just as well as a normal comment (in my opinion). And with normal comments you get the date it was written as well, so that you can more easily see if it's updated info or not. Espescially when we don't just delete the data in the admin_comment field completely, but rather mighrated it to a comment, I feel like it is completly fine. I don't have access to to itDAGENES admin page, how do they do it?

@juniwbjerde juniwbjerde force-pushed the remove-adminComment-from-companies branch from cfa6fed to 2948c1d Compare October 9, 2024 19:17
jonasdeluna and others added 17 commits October 9, 2024 21:40
…timestamp

Add company contact update timestamp
Well, almost unused. Replaced the usage in the response of
reset_password with CurrentUserSerializer
…alizer

Remove unused DetailedUserSerializer
Bumps [django-cors-headers](https://github.com/adamchainz/django-cors-headers) from 3.14.0 to 4.5.0.
- [Changelog](https://github.com/adamchainz/django-cors-headers/blob/main/CHANGELOG.rst)
- [Commits](adamchainz/django-cors-headers@3.14.0...4.5.0)

---
updated-dependencies:
- dependency-name: django-cors-headers
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [django-redis](https://github.com/jazzband/django-redis) from 5.2.0 to 5.4.0.
- [Release notes](https://github.com/jazzband/django-redis/releases)
- [Changelog](https://github.com/jazzband/django-redis/blob/master/CHANGELOG.rst)
- [Commits](jazzband/django-redis@5.2.0...5.4.0)

---
updated-dependencies:
- dependency-name: django-redis
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [black](https://github.com/psf/black) from 24.8.0 to 24.10.0.
- [Release notes](https://github.com/psf/black/releases)
- [Changelog](https://github.com/psf/black/blob/main/CHANGES.md)
- [Commits](psf/black@24.8.0...24.10.0)

---
updated-dependencies:
- dependency-name: black
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [django](https://github.com/django/django) from 4.0.10 to 4.2.16.
- [Commits](django/django@4.0.10...4.2.16)

---
updated-dependencies:
- dependency-name: django
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
norbye and others added 24 commits December 16, 2024 09:04
Bumps [jinja2](https://github.com/pallets/jinja) from 3.1.4 to 3.1.5.
- [Release notes](https://github.com/pallets/jinja/releases)
- [Changelog](https://github.com/pallets/jinja/blob/main/CHANGES.rst)
- [Commits](pallets/jinja@3.1.4...3.1.5)

---
updated-dependencies:
- dependency-name: jinja2
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
* Add pagination, ordering to AdminCompanyViewSet

* Format code

* Add migrations
…ng-meetings

Prevent admins from searching meetings
Improve feedback for abacard responses
Add readme to image before running install
Lock poetry version to 1.8.5
* Add leaderboard for overall score

* Update achievement score, make percentage cached

* Update achievement_score

* Implement pagination achievements

* Fix tests
…es' into remove-adminComment-from-companies

# Conflicts:
#	lego/apps/companies/serializers.py
@Bestem0r
Copy link
Contributor

Bestem0r commented Feb 5, 2025

Welp. I kinda messed up this PR in order to hijack it and get it merged.... 😅

@ivarnakken
Copy link
Member

I might actually die if this is not squashed

@ivarnakken
Copy link
Member

Actually, no, do not merge this before fixing the commits.
image

@norbye
Copy link
Member

norbye commented Feb 6, 2025

I think he recreated it in #3727, so this PR could be closed and abandoned

@ivarnakken
Copy link
Member

ah i didnt catch that haha - nice then!

@ivarnakken ivarnakken closed this Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Pull requests or issues that need discussion, e.g. regarding controversial/big changes review-needed Pull requests that need review small-fix Pull requests that fix something small
Projects
None yet
Development

Successfully merging this pull request may close these issues.