public
/
pyceo
25
1
Fork 2
Code Issues 14 Pull Requests 2 Projects Releases 1 Wiki Activity
Labels Milestones

Implement TUI support for multiple users in each position #80

Merged
merenber merged 6 commits from feature-59 into master 2 months ago
Conversation 0 Commits 6 Files Changed 14
j24chung commented 6 months ago
Collaborator
There is no content yet.
j24chung added 1 commit 6 months ago
j24chung added 1 commit 6 months ago
continuous-integration/drone/pr Build is passing
Details
d16e7bb293
Fixed linting
r389li requested changes 6 months ago
r389li left a comment
Owner

Almost there! Thanks for taking this on!

Almost there! Thanks for taking this on!
ceo/tui/controllers/SetPositionsController.py Outdated
for pos, field in self.view.position_fields.items():
if field.edit_text != '':
body[pos] = field.edit_text
logger.info(body)
r389li commented 6 months ago
Owner

Will this clutter up log?

Will this clutter up log?
j24chung commented 6 months ago
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.
j24chung commented 6 months ago
Poster
Collaborator

Fixed :)

Fixed :)
j24chung marked this conversation as resolved
ceod/transactions/members/UpdateMemberPositionsTransaction.py Outdated
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")
r389li commented 6 months ago
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!
j24chung commented 6 months ago
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
j24chung commented 6 months ago
Poster
Collaborator

I could def add tests targeting this new feature!

I could def add tests targeting this new feature!
r345liu commented 4 months ago
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 6 months ago
continuous-integration/drone/pr Build is failing
Details
910449d67a
Remove some logging code
j24chung added 1 commit 6 months ago
continuous-integration/drone/pr Build is passing
Details
4145491373
Fix linting error
merenber added 2 commits 2 months ago
merenber merged commit 5e8f1b5ba5 into master 2 months ago
merenber deleted branch feature-59 2 months ago

Reviewers

r389li requested changes 6 months ago
continuous-integration/drone/pr Build is passing
Details
The pull request has been merged as 5e8f1b5ba5.
Sign in to join this conversation.
Reviewers
Request review
No reviewers
r389li
Labels
Apply labels
Clear labels
No items
No Label
Milestone
Set milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Assign users
Clear assignees
No Assignees
4 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: public/pyceo#80
Write Preview
Loading…
Cancel
Save
There is no content yet.
Delete Branch 'feature-59'

Deleting a branch is permanent. It CANNOT be undone. Continue?

No
Yes