Skip to content
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

Added support to render widgets partial without any layout furniture #3989

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

abujeda
Copy link
Contributor

@abujeda abujeda commented Dec 4, 2024

Added support to render widgets partials without any layout furniture.

This change will allow for better support for OOD extensions in the form of widgets and the support of dynamic content with Javascript with Widgets.

Fixes #3986

@johrstrom
Copy link
Contributor

This look OK - I just have to take a second to consider the security of it.

@@ -106,6 +106,8 @@

post 'settings', :to => 'settings#update'

match '/widgets/*widget_path', to: 'widgets#show', via: [:get, :post], as: 'widgets'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay on this. Why do we need post here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for completeness. There might be complex widgets that might require to submit data from the frontend to the backend for an update of the state/data of the widget after the initial render. Adding a POST will provide a more REST friendly way of getting the data.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for completeness...

We don't seem to be passing the body or query params to the partial. That seems super risky to do so, because we don't know them up front and so can't filter them. I have to admit that scares me a bit.

Copy link
Contributor Author

@abujeda abujeda Dec 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, there is no need to pass them and this is not adding something that is not already possible. The body and query params are available in any template through the params object.

This is a valid and working widget template (tested locally to confirm)
/pun/sys/dashboard?parameter=Aday

# /pun/sys/dashboard?parameter=Aday 
# widget template
<%
  value = params[:parameter]
%>
<%= "Query Param Value: #{value}" %>

Doing this with a POST will be more REST friendly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To mitigate the security concerns, we could make this a experimental feature for now to give us time to try and discover any issues.

# Experimental Feature
# Allows widget partials to be rendered without any page furniture.
# It can be use to extend OOD functionality.
  if Configuration.widget_partials_enabled?
     match '/widgets/*widget_path', to: 'widgets#show', via: [:get, :post], as: 'widgets'
  end

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To mitigate the security concerns, we could make this a experimental feature

Yes please. I just tagged 4.0.0 but this can make it into 4.0.1.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes completed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Widgets - Add support to get widgets partial HTML
3 participants