Implement TUI support for multiple users in each position #80

Merged
merenber merged 6 commits from feature-59 into master 2 weeks ago
Collaborator
There is no content yet.
j24chung added 1 commit 4 months ago
j24chung added 1 commit 4 months ago
d16e7bb293
Fixed linting
r389li requested changes 4 months ago
r389li left a comment
Owner

Almost there! Thanks for taking this on!

Almost there! Thanks for taking this on!
for pos, field in self.view.position_fields.items():
if field.edit_text != '':
body[pos] = field.edit_text
logger.info(body)
Owner

Will this clutter up log?

Will this clutter up log?
Poster
Collaborator

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.
Poster
Collaborator

Fixed :)

Fixed :)
j24chung marked this conversation as resolved
for user in username:
self.positions[user].append(position)
else:
raise TypeError("Username(s) under each position must either be a string or a list")
Owner

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!
Poster
Collaborator

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
Poster
Collaborator

I could def add tests targeting this new feature!

I could def add tests targeting this new feature!
Owner

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
j24chung added 1 commit 4 months ago
910449d67a
Remove some logging code
j24chung added 1 commit 4 months ago
4145491373
Fix linting error
merenber added 2 commits 2 weeks ago
merenber merged commit 5e8f1b5ba5 into master 2 weeks ago
merenber deleted branch feature-59 2 weeks ago

Reviewers

r389li requested changes 4 months ago
continuous-integration/drone/pr Build is passing
The pull request has been merged as 5e8f1b5ba5.
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No project
No Assignees
4 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: public/pyceo#80
Loading…
There is no content yet.