Skip to content

Adding the Manage Members Page - #649

Open
brightfietsop-ux wants to merge 99 commits into
department-portal-basefrom
Bright_Artem_manage_people
Open

Adding the Manage Members Page#649
brightfietsop-ux wants to merge 99 commits into
department-portal-basefrom
Bright_Artem_manage_people

Conversation

@brightfietsop-ux

@brightfietsop-ux brightfietsop-ux commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

We have added the Manage Members page, which can be accessed through the Members card on the Department Portal.

WHAT'S WORKING:

  • The names of supervisors are correctly pulled from the demo data.
  • Supervisees are properly displayed (on the table, active means the student has approved their labor status form, and - pending means they have not done that yet).
  • The Assign Coordinator switch works!
  • The Ban/Unban Button also works!
  • You can remove members by clicking on the trash icon to the right
  • The Add Members dropdown/button technically works.
  • The Ban/Unban button and the Remove Member button are displayed ONLY FOR ADMINS (as it is supposed to be).

WHAT'S NOT WORKING:

  • we cannot directly access the page using the manage member button
  • When supervisors are assigned as LC, they get assigned in every department.
  • We cannot add department members who are not supervisors
  • All staff members are supervisors

@ArtemKurasov ArtemKurasov changed the title Bright artem manage people Adding the Manage Members Page Jul 20, 2026
@BhushanSah

Copy link
Copy Markdown
Contributor

I added tests for the new Manage Members helper logic.

The tests cover the smaller helper functions separately so the behavior is easier to verify and debug:

  • supervisorsDbToDict

    • Checks that supervisor data is converted into the expected dictionary format.
    • Verifies that extra spaces are stripped from fields.
    • Confirms that the username is created correctly from the supervisor email.
  • currentAcademicYear

    • Tests the academic year calculation before July.
    • Tests the academic year calculation starting in July.
    • I used monkeypatch so the test does not depend on the actual current date.
  • attachPositionCounts

    • Tests that active/pending primary and secondary counts are added correctly when count data exists.
    • Tests that missing count data defaults to 0, so the Manage Members page does not break when a supervisor has no matching labor forms.
  • getStudentCounts

    • Tests that active primary, pending primary, active secondary, and pending secondary positions are counted correctly.
    • Uses test department, supervisor, student, term, and labor status form records to verify the actual database query behavior.
  • Released labor forms

    • Added a test to confirm that released labor forms are excluded from the active/pending position counts.
    • This is important because the Manage Members page should not count positions that have already been released.

I marked the database-dependent tests with @pytest.mark.integration, while the smaller pure helper tests were left as normal unit tests because they do not touch the database or Flask session.

All tests in tests/code/test_manageMembers.py are passing.

Comment thread app/templates/main/managepositions.html Outdated
Comment thread app/templates/main/supervisorPortal.html Outdated
Comment thread database/reset_database.sh.bak Outdated
Comment thread database/reset_database.sh Outdated
Comment thread db_test.py
Comment thread app/static/js/addSupervisorsToDepartment.js
Comment thread app/static/css/managepositions.css Outdated
Comment thread app/models/supervisor.py
Comment thread app/logic/tracy.py Outdated

@MImran2002 MImran2002 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As you can see here I created a form under wario nakazawa

Image there is no approve form but it is showing active here

Image eventhough wario has no approved form yet.

But when i look there is a approved secondary for Elehah for secondary so that might influence the result. Moreover, there is a problem where

image as you can see scott has active secondary but in the manage member it only shows pending.

Comment thread app/static/css/manageMembers.css Outdated
Comment thread app/static/css/manageMembers.css Outdated

@MImran2002 MImran2002 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

eventhough it says 2026-2027 the active and pending that are there shows those of 2025-2026

Image

@MImran2002 MImran2002 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

eventhough it says 2026-2027 the active and pending that are there shows those of 2025-2026

Image

@BhushanSah

Copy link
Copy Markdown
Contributor

As you can see here I created a form under wario nakazawa

Image there is no approve form but it is showing active here

Image eventhough wario has no approved form yet.

But when i look there is a approved secondary for Elehah for secondary so that might influence the result. Moreover, there is a problem where

image as you can see scott has active secondary but in the manage member it only shows pending.

Fixed this by changing the Manage Members count logic to use the Labor Status Form status from FormHistory instead of studentConfirmation. Approved forms now count as active, Pending and Pre-Student Approval forms count as pending, and denied/released forms are ignored. I also filtered the counts to the current academic year so forms from AY 2025-2026 do not show under the AY 2026-2027 page.

Comment thread app/controllers/main_routes/__init__.py Outdated
Comment thread app/controllers/main_routes/departmentPortal.py Outdated
Comment thread app/controllers/main_routes/departmentPortal.py Outdated
Comment thread app/controllers/main_routes/departmentPortal.py Outdated
Comment thread app/controllers/main_routes/departmentPortal.py
Comment thread app/__init__.py Outdated
Comment thread tests/code/test_departmentPortal.py Outdated
Comment thread tests/code/test_manageMembers.py Outdated
Comment thread tests/code/test_manageMembers.py Outdated
Comment thread tests/code/test_manageMembers.py
@BhushanSah

Copy link
Copy Markdown
Contributor

I addressed the latest review feedback across the Manage Members PR. I cleaned up the imports, moved currentYear to g.currentYear using the same before_request pattern as g.currentUser, restored the positions route so this PR stays within Manage Members scope, renamed the count helper to getActivePendingPositionCounts, updated the route variable names to be more descriptive, and refactored the tests so each function has one test named test_functionName with scenarios handled inside that test.

I also fixed the broken test by matching the helper signature and adding FormHistory rows for the Labor Status Form scenarios, so the test follows the actual count logic. I reran:

pytest tests/code/test_departmentPortal.py tests/code/test_manageMembers.py -q

and the tests pass locally.

@BhushanSah

Copy link
Copy Markdown
Contributor

@MImran2002
Fixed the stale session issue that happened when switching users without clearing cookies.

The problem was that the app was reusing the cached currentUser from the Flask session. So if someone logged in as a Labor Admin, logged out, and then logged in as a student, the session could still contain the old Labor Admin user. That allowed the student to access Manage Members until the cookies were cleared. The same issue also happened in reverse, where logging back in as a Labor Admin could still show the old student permissions.

I fixed this by checking the username from the current request against the cached currentUser in the session. If they match, the app keeps using the cached user. If they do not match, the old currentUser and username are removed from the session and the correct user is loaded again through require_login.

I also updated require_login so session['username'] is refreshed when the logged-in user changes.

The focused Manage Members and Department Portal tests pass locally with 5 passed.

@BhushanSah
BhushanSah force-pushed the Bright_Artem_manage_people branch from 83b34d4 to 35dbd4d Compare August 4, 2026 20:38
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.

6 participants