task/WP-109-remove-unused-django-fields #887

tjgrafft · 2023-10-16T18:08:07Z

Overview

Many fields connected to the PortalProfile model in the Django backend are no longer used by the portal. Remove all that are no longer needed from the model, and include related migrations.

Changes

Removed several fields from PortalProfile model on django backend that were not being used in the CEP anymore (bio, website, orcid_id, etc)
Removed gender and ethnicity table headers from table on Manage Account page of CEP
Removed code from the related unit tests that were no longer needed bc fields were removed.
Removed code in fixtures that was testing for the unnecessary fields, as well.
Removed the two fields in related migrations file too.

Testing

Log in to CEP
Go to your username at the top right and open the dropdown
Click on "My Account"
View should match table format below. No ethnicity nor gender table header/value should be present
Also check https://cep.test/accounts/api/profile/data/
There should be no bio, orcid_id, professional_lvel, website, gender, or ethnicity key/value pairs in the api response on that page.

UI

Notes

codecov · 2023-10-16T18:10:00Z

Codecov Report

Merging #887 (1c30780) into main (f12180a) will increase coverage by 6.34%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #887      +/-   ##
==========================================
+ Coverage   63.33%   69.67%   +6.34%     
==========================================
  Files         427      220     -207     
  Lines       12245     6111    -6134     
  Branches     2514     1812     -702     
==========================================
- Hits         7755     4258    -3497     
+ Misses       4284     1779    -2505     
+ Partials      206       74     -132

Flag	Coverage Δ
javascript	`69.67% <ø> (ø)`
unittests	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...c/components/ManageAccount/ManageAccountTables.jsx	`76.00% <ø> (ø)`

... and 207 files with indirect coverage changes

chandra-tacc

Requested changes: remove all the unused code.

Question: edit profile leads to https://accounts.tacc.utexas.edu/profile, which shows the deleted fields. Is this ok?

Also, ran migrations locally:

root@07dc05f0e902:/srv/www/portal/server# python3 manage.py makemigrations
Migrations for 'accounts':
  portal/apps/accounts/migrations/0006_auto_20231017_1944.py
    - Remove field bio from portalprofile
    - Remove field orcid_id from portalprofile
    - Remove field professional_level from portalprofile
    - Remove field website from portalprofile
root@07dc05f0e902:/srv/www/portal/server# python3 manage.py migrate
Operations to perform:
  Apply all migrations: accounts, admin, auth, contenttypes, datafiles, googledrive, impersonate, notifications, onboarding, portal_auth, portal_licenses, portal_messages, projects, sessions, termsandconditions, webhooks, workspace
Running migrations:
  Applying accounts.0006_auto_20231017_1944... OK

chandra-tacc · 2023-10-17T19:39:54Z

client/src/components/ManageAccount/ManageAccountTables.jsx

@@ -65,8 +65,8 @@ export const ProfileInformation = () => {
      { Header: 'Institution', accessor: 'institution' },
      { Header: 'Country of Residence', accessor: 'country' },
      { Header: 'Country of Citizenship', accessor: 'citizenship' },
-      { Header: 'Ethnicity', accessor: 'ethnicity' },
-      { Header: 'Gender', accessor: 'gender' },
+      // { Header: 'Ethnicity', accessor: 'ethnicity' },


can we remove these instead of commenting them oout

chandra-tacc · 2023-10-17T19:40:12Z

client/src/components/ManageAccount/tests/ManageAccountTables.test.js

-      website: 'http://owais.io',
-      orcid_id: 'test',
-      professional_level: 'Staff (support, administration, etc)',
+      // ethnicity: 'Asian',


same here, delete this unused code

Good question, I believe those values (on https://accounts.tacc.utexas.edu/profile) are different than the values that we're deleting on the backend. I say this because when I look at the api response on https://cep.test/accounts/api/profile/data/, those values are either empty strings or null for my profile, whereas on https://accounts.tacc.utexas.edu/profile, they are filled out. So my guess is this is separate from the PortalProfile data.

rstijerina

Changes look good! Just want to undo that change to the init migration

rstijerina · 2023-10-18T16:44:35Z

server/portal/apps/accounts/migrations/0001_initial.py

-                ('ethnicity', models.CharField(max_length=255)),
-                ('gender', models.CharField(max_length=255)),


We want to keep old migrations unaltered, and rather all our future changes can be handled by additional migrations

chandra-tacc

LGTM

chandra-tacc · 2023-10-18T19:18:16Z

@tjgrafft - server side unit tests are failing. Once this is fixed and tests pass, I can merge.

ERROR portal/apps/webhooks/unit_test.py::TestInteractiveWebhookView::test_unsupported_event_type - django.db.utils.IntegrityError: Problem installing fixture '/home/runner/work/Core-Portal/Core-Portal/server/portal/fixtures/users.json': Could not load accounts.PortalProfile(pk=1): NOT NULL constraint failed: accounts_portalprofile.ethnicity

tjgrafft · 2023-10-18T19:37:49Z

Should be fixed now with this new commit

task/WP-109-remove-unused-django-fields-v1

ad63630

chandra-tacc requested changes Oct 17, 2023

View reviewed changes

tjgrafft and others added 2 commits October 17, 2023 15:20

Merge branch 'main' into task/WP-109-remove-unused-django-fields

baa2ab4

task/WP-109-remove-unused-django-fields-v2

8f8c30c

tjgrafft requested review from chandra-tacc and rstijerina October 17, 2023 20:46

tjgrafft changed the title ~~task/WP-109-remove-unused-django-fields-v1~~ task/WP-109-remove-unused-django-fields Oct 17, 2023

rstijerina requested changes Oct 18, 2023

View reviewed changes

tjgrafft and others added 2 commits October 18, 2023 13:20

Merge branch 'main' into task/WP-109-remove-unused-django-fields

bc205f1

task/WP-109-remove-unused-django-fields-v3

89c4b29

rstijerina approved these changes Oct 18, 2023

View reviewed changes

chandra-tacc approved these changes Oct 18, 2023

View reviewed changes

task/WP-109-remove-unused-django-fields-v4

5dee19a

Merge branch 'main' into task/WP-109-remove-unused-django-fields

1c30780

rstijerina merged commit e427c97 into main Oct 24, 2023
6 checks passed

rstijerina deleted the task/WP-109-remove-unused-django-fields branch October 24, 2023 18:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

task/WP-109-remove-unused-django-fields #887

task/WP-109-remove-unused-django-fields #887

tjgrafft commented Oct 16, 2023

codecov bot commented Oct 16, 2023 •

edited

Loading

chandra-tacc left a comment

chandra-tacc Oct 17, 2023

chandra-tacc Oct 17, 2023

tjgrafft Oct 17, 2023 •

edited

Loading

rstijerina left a comment

rstijerina Oct 18, 2023

chandra-tacc left a comment

chandra-tacc commented Oct 18, 2023

tjgrafft commented Oct 18, 2023

		('ethnicity', models.CharField(max_length=255)),
		('gender', models.CharField(max_length=255)),

task/WP-109-remove-unused-django-fields #887

task/WP-109-remove-unused-django-fields #887

Conversation

tjgrafft commented Oct 16, 2023

Overview

Related

Changes

Testing

UI

Notes

codecov bot commented Oct 16, 2023 • edited Loading

Codecov Report

chandra-tacc left a comment

Choose a reason for hiding this comment

chandra-tacc Oct 17, 2023

Choose a reason for hiding this comment

chandra-tacc Oct 17, 2023

Choose a reason for hiding this comment

tjgrafft Oct 17, 2023 • edited Loading

Choose a reason for hiding this comment

rstijerina left a comment

Choose a reason for hiding this comment

rstijerina Oct 18, 2023

Choose a reason for hiding this comment

chandra-tacc left a comment

Choose a reason for hiding this comment

chandra-tacc commented Oct 18, 2023

tjgrafft commented Oct 18, 2023

codecov bot commented Oct 16, 2023 •

edited

Loading

tjgrafft Oct 17, 2023 •

edited

Loading