Add conference partners #4251
Add conference partners #4251
Conversation
ghost
assigned 
|
@decidim/lot-core this PR is ready to review 馃槃 |
|
@rbngzlv could you add some screenshots too? |
|
@oriolgual I'will added |
| private | ||
|
|
||
| def partner | ||
| return block unless model.link.presence |
There was a problem hiding this comment.
Maybe some of this view logic should go to the template, not inside the cell? It feels weird to me having all the markup here instead of the template.
| def initialize(form, current_user, conference) | ||
| @form = form | ||
| @current_user = current_user | ||
| @conference = conference |
There was a problem hiding this comment.
Shouldn't the conference be part of the form too?
There was a problem hiding this comment.
why? We are doing the same with speakers, conference admins, assembly admins, assembly members...
There was a problem hiding this comment.
I think it's cleaner (you don't have to merge attributes from the form) but maybe I didn't review it correctly in other PRs.
There was a problem hiding this comment.
I still think that it is better to continue with the same system, for the whole decidim. Not in different ways according to the command...
There was a problem hiding this comment.
I agree to what @isaacmg410 says! 馃槃
| # conference_partner - The ConferencePartner to update | ||
| def initialize(form, conference_partner) | ||
| @form = form | ||
| @conference_partner = conference_partner |
There was a problem hiding this comment.
Same as the creation form, if I'd move this inside the form so the command doesn't need to check anything else.
There was a problem hiding this comment.
In this case, is conference_partner, not conference.
|
|
||
| def edit | ||
| @partner = collection.find(params[:id]) | ||
| enforce_permission_to :update, :partner, speaker: @partner |
There was a problem hiding this comment.
Shouldn't this be partner instead of speaker?
|
|
||
| def update | ||
| @partner = collection.find(params[:id]) | ||
| enforce_permission_to :update, :partner, speaker: @partner |
There was a problem hiding this comment.
Shouldn't this be partner instead of speaker?
|
|
||
| def destroy | ||
| @partner = collection.find(params[:id]) | ||
| enforce_permission_to :destroy, :partner, speaker: @partner |
There was a problem hiding this comment.
Shouldn't this be partner instead of speaker?
| def index | ||
| enforce_permission_to :index, :partner | ||
|
|
||
| @query = params[:q] |
There was a problem hiding this comment.
nowhere, deleted!
|
@oriolgual requested changes applied! |
|
Merging! Tests look like they are not passing, but they really passed: https://circleci.com/workflow-run/a6af249c-e498-48fd-b200-077d298d00e4 |
馃帺 What? Why?
This PR adds the new functionality of Conference Partners. Is part of the #3709
馃搶 Related Issues
馃搵 Subtasks
CHANGELOGentry馃摲 Screenshots (optional)