administrate
administrate copied to clipboard
Add context to Page
I have some proposal regarding Issue #2363. I believe this PR can still be improved, so if anyone has any opinions, please review and let me know.
Administrate::Page#context
By keeping the context in the Page as follows,
page = Administrate::Page::Form.new(dashboard, requested_resource)
page.context = self # -> current controller
We can now define it as follows in the Dashboard.
ATTRIBUTE_TYPES = {
catetory: Field::BelongsTo.with_options(
scope: -> (field) { field.page.context.current_user.organization.categories }
),
With this, we can now differentiate the BelongsTo options based on the current user's permissions.
Also, by using context in form_attributes like this, we can now differentiate form elements based on the current user's permissions.
def form_attributes(action = nil, context = nil)
if ["new", "create"].include?(action.to_s) && context.try(:pundit_user).try(:admin?)
super
else
super - [:customer]
end
end
Administrate::ApplicationController#contextualize_resource
I've prepared a hook point to add context to the resource. This allows us to automatically set the current_user(pundit_user) to the customer form element that was omitted in the previous section.
def contextualize_resource(resource)
if ["new", "create"].include?(action_name) && !pundit_user.admin?
resource.customer = pundit_user
end
end
Also, we can change the context here for models where the content of the validation changes under certain conditions.
class User < ApplicationRecord
attr_accessor :inviting_by_admin
validate :validate_invitation_code_matching, on: :create, unless: -> { inviting_by_admin.present? }
end
def contextualize_resource(resource)
resource.inviting_by_admin = true if action_name == "create"
end
How do you think about?
Is it better to create Administrate::Page::Context because the contents of the context are too free and it's concerning?
As mentioned in comments above, I have created a branch that uses your code and I'm experimenting with it. It's at https://github.com/thoughtbot/administrate/compare/main...pablobm:use-cases-for-contextualize. My goals are:
- Work with your proposal so see how comfortable I am with the decisions.
- See how it can address the issues listed at https://github.com/thoughtbot/administrate/issues/2363, and what changes would be necessary in order to tackle all of them.
I think this is the way to go, but there are some things still to polish. Let's see if I can make sense.
The main issue is https://github.com/thoughtbot/administrate/issues/2363. This lists several other issues that require context. Therefore when we look for a solution we should find something that can tackle all the presented use cases, or at least enable a path towards tackling them in the future. Let's look at them one by one.
https://github.com/thoughtbot/administrate/issues/1342 How to scope dropdown options to current_user in forms
I think this is addressed here, but the issue covers only part of the story. The proposed change tackles BelongsTo, but this should also extend to HasMany and HasOne. In fact, now I'm thinking it should extend to all field types, as I can envision similar issues with Select and possibly others.
https://github.com/thoughtbot/administrate/issues/1586 Support for virtual fields (regression)
I'm not sure this one is actually related :sweat_smile: In any case, if all fields can get a context, then this one will benefit too.
https://github.com/thoughtbot/administrate/issues/1862 Restricting specific fields visible in show/edit
There are several things here:
- Show different attributes depending on context: this works for forms, but I think not for
showandindex. Let's add that too. - Ability to set certain fields depending on the context: I think this changes enables this behaviour by passing the context to
permitted_attributes. - Same as 1, but for associations. I'm having a look and that might be tricky though. Not sure.
https://github.com/thoughtbot/administrate/issues/1988 Filtering Field::HasMany
Already mentioned.
https://github.com/thoughtbot/administrate/issues/2060 valid_actions? doesn't work for has_many relations
I think the context on permitted_attributes would help here too...?
Please let me know what you think!
#1342 How to scope dropdown options to current_user in forms #1988 Filtering Field::HasMany
While it is possible to use the scope in Lambda format that administrate-field-scoped_belongs_to and administrate-field-scoped_has_many already introduce, it would be great to have the scope option available in Field::BelongsTo and Field::HasMany as well, considering the high demand for this feature.
Field::Select already has a collection option in Lambda format, so I believe it can still be used as-is even with the introduction of context.
Personally, I would be happy to see similar support for administrate-field-nested_has_many as well.
#1862 Restricting specific fields visible in show/edit
I have another suggestion regarding this.
In the edit screen, there are cases where users don't have permission to edit certain data but still need to see it.
For example, a BelongsTo field might be set by higher-level administrators and cannot be edited by regular administrators. However, not displaying this field in the edit screen would negatively affect usability. In the demo application, if we set the Customer field in the /orders/:id/edit screen to be hidden for regular administrators, and it disappears from the edit screen, users will not understand what they are trying to edit.
Do you have any ideas for mixing the _form partial and the _show partial in the edit screen? Of course, I would like to utilize context for this as well.
#2060 valid_actions? doesn't work for has_many relations
Isn't this the same issue as specific fields restrictions?
I think it would be sufficient to just add context to permitted_attributes, but it would be nice to have a more concise way to describe it.
Could Pundit's permitted_attributes_for_#{action} be a useful reference?
https://github.com/varvet/pundit/blob/main/lib/pundit/authorization.rb#L128
I think we are in agreement here:
- Yes to adding
scopeto all associative fields, as well as context tocollectioninSelect. - Very good point about showing read-only fields in forms.
- I wonder if this could be inferred from the
permit_attributes_for_#{action}that you propose. - So for example:
Order#customeris listed inFORM_ATTRIBUTES, but not allowed inpermit_attributes_for_edit, so Administrate knows to show it read-only. Would that work? Would that be too complex or clever? I'll let you explore :-)
- I wonder if this could be inferred from the
- And indeed,
permit_attributes_for_#{action}might be good for us here. I'll let you explore this one too!
@pablobm Is there anything else I can help with moving forward? If the scope of this PR is decided, I'm ready to commit to it until the end.
@goosys - I think you are good to go, thank you! :+1: If it helps, you can let me know as you go and I can keep try keep an eye on your pushes to see how you are doing.