public/pyceo
30
1
Fork 6
Code Issues 26 Pull Requests 6 Projects Releases 1 Wiki Activity

Implement TUI support for multiple users in each position #80

Merged
merenber merged 6 commits from feature-59 into master 2023-01-23 02:26:15 -05:00
Conversation 1 Commits 6 Files Changed 14 +8 -3
3 changed files with 8 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit d16e7bb293 - Show all commits

5
ceo/tui/controllers/SetPositionsController.py
View File

@ -8,6 +8,10 @@ import ceo.tui.utils as tui_utils
from ceo.tui.views import TransactionView
from ceod.transactions.members import UpdateMemberPositionsTransaction
from ceo_common.logger_factory import logger_factory
logger = logger_factory(__name__)
class SetPositionsController(Controller):
def __init__(self, model, app):
@ -18,6 +22,7 @@ class SetPositionsController(Controller):
for pos, field in self.view.position_fields.items():
if field.edit_text != '':
body[pos] = field.edit_text
logger.info(body)
j24chung marked this conversation as resolved Outdated
r389li commented 2022-10-13 23:47:42 -04:00
Outdated
Review
Copy Link

Will this clutter up log?

Will this clutter up log?
j24chung commented 2022-10-14 13:12:44 -04:00
Outdated
Review
Copy Link

Not sure why but this piece of code isn't supposed to be in the branch anymore. I don't see it on my local repo which should be in sync with the branch being merged. I'll look into why it isn't gone yet.

Not sure why but this piece of code isn't supposed to be in the branch anymore. I don't see it on my local repo which should be in sync with the branch being merged. I'll look into why it isn't gone yet.
j24chung commented 2022-10-14 13:21:58 -04:00
Outdated
Review
Copy Link

Fixed :)

Fixed :)
model = TransactionModel(
UpdateMemberPositionsTransaction.operations,
'POST', '/api/positions', json=body

2
ceod/api/positions.py
View File

@ -35,7 +35,7 @@ def update_positions():
# remove falsy values and parse multiple users in each position
# Example: "user1,user2, user3" -> ["user1","user2","user3"]
body = {
positions: username.replace(' ','').split(',') for positions, username in body.items()
positions: username.replace(' ', '').split(',') for positions, username in body.items()
if username
}

4
ceod/transactions/members/UpdateMemberPositionsTransaction.py
View File

@ -21,7 +21,7 @@ class UpdateMemberPositionsTransaction(AbstractTransaction):
'subscribe_to_mailing_lists',
]
def __init__(self, positions_reversed: Dict[str, Union[str,list]]):
def __init__(self, positions_reversed: Dict[str, Union[str, list]]):
# positions_reversed is position -> username
super().__init__()
self.ldap_srv = component.getUtility(ILDAPService)
@ -36,7 +36,7 @@ class UpdateMemberPositionsTransaction(AbstractTransaction):
self.positions[user].append(position)
else:
raise TypeError("Username(s) under each position must either be a string or a list")
r389li commented 2022-10-14 00:24:41 -04:00
Outdated
Review
Copy Link

Would the rest of this file work as well? On a cursory glance there's a couple more places that look like they should need changing (i.e. removing old positions, etc.)

Also, let's add some tests specifically targetting this, so we can future-proof guarantees of it working!

Would the rest of this file work as well? On a cursory glance there's a couple more places that look like they should need changing (i.e. removing old positions, etc.) Also, let's add some tests specifically targetting this, so we can future-proof guarantees of it working!
j24chung commented 2022-10-14 13:29:41 -04:00
Outdated
Review
Copy Link

Could you clarify on what places need more changing? Not very sure about the whole code design but all I've changed is that when a position has a list of userids, instead of just a string of one user's userid, it will know how to parse that list and add the position to each user. I assume the remaining code knows how to handle a case where one user has multiple positions, as required

Could you clarify on what places need more changing? Not very sure about the whole code design but all I've changed is that when a position has a list of userids, instead of just a string of one user's userid, it will know how to parse that list and add the position to each user. I assume the remaining code knows how to handle a case where one user has multiple positions, as required
j24chung commented 2022-10-14 13:30:36 -04:00
Outdated
Review
Copy Link

I could def add tests targeting this new feature!

I could def add tests targeting this new feature!
r345liu commented 2022-11-26 20:49:28 -05:00
Outdated
Review
Copy Link

I think this is the only other place that needs to be updated


            self.old_positions[username] = old_positions
            for position in new_positions:
                new_positions_reversed[position] = username

Also, I think the ceod api is the only place using UpdateMemberPositionsTransaction, so you can just change the type without needing to care about string

I think this is the only other place that needs to be updated ```python self.old_positions[username] = old_positions for position in new_positions: new_positions_reversed[position] = username ``` Also, I think the ceod api is the only place using `UpdateMemberPositionsTransaction`, so you can just change the type without needing to care about string
# a cached Dict of the Users who need to be modified (username -> User)
self.users: Dict[str, IUser] = {}