-
-
Notifications
You must be signed in to change notification settings - Fork 449
An assembly member can be an existing user #3302
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| attribute :position_other, String | ||
| attribute :user_id, Integer | ||
| attribute :existing_user, Boolean, default: false | ||
| attribute :user_id, Integer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is duplicated
|
|
||
| query = current_organization.users&.order(name: :asc) | ||
| term = params[:term] | ||
| if term&.start_with?("@") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've seen this code in other PRs, should we extract this to a class or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yo've seen it in the same module? If not, we need to extract it to the core module so we don't add a dependency to the admin, right? We can extract it to a query class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oriolgual, you refer to this other PR that we are "arguing" there. #3136 (comment)
We can look for a solution for both systems. What do you recommend?
Create a OrganizersController or UsersController but should be done in Decidim::Core no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, should be done at core, no problem. I'd go with a OrganizersController
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good! But in this case, will be UsersController ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, yes you're right
955f457 to
8750a8b
Compare
8750a8b to
714dd92
Compare
30c0900 to
c228ab0
Compare
714dd92 to
31d1f22
Compare
|
@decidim/lot-core review it please, rebased on master after merging #3387. |
🎩 What? Why?
This PR sits on #3008 and #3301 and add the possibility to link an assembly member to an existing user in the platform.
I'm pointing this PR to the
feature/assembly_membersbranch so you can review it faster, once merged, I'll point this to the master branch.📌 Related Issues
📋 Subtasks
CHANGELOGentry