Skip to content

Commit

Permalink
Merge branch 'master' into feature/3985-PM-manifest-support-for-copie…
Browse files Browse the repository at this point in the history
…d-files
  • Loading branch information
euler-room committed Jan 14, 2025
2 parents b66e132 + 2456c97 commit 76da4f1
Show file tree
Hide file tree
Showing 22 changed files with 288 additions and 127 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

## [Unreleased]

### Fixed
- Icon picker correctly shows all icons when the search string is empty in [4065](https://github.com/OSC/ondemand/pull/4065).
- Batch connect cards correctly display cores in [4057](https://github.com/OSC/ondemand/pull/4057).
- Icon picker correctly shows and hides the spinner in [4051](https://github.com/OSC/ondemand/pull/4051).
- Project manager template selection fixed in [4054](https://github.com/OSC/ondemand/pull/4054).

### Added
- Added support to render widgets partial without any layout furniture in [3989](https://github.com/OSC/ondemand/pull/3989).

## [4.0.0] - 12-30-2024

### Added
Expand Down
3 changes: 2 additions & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ RUN dnf -y install https://yum.osc.edu/ondemand/latest/ondemand-release-web-late
RUN dnf -y update && \
dnf install -y dnf-utils && \
dnf config-manager --set-enabled powertools && \
dnf -y module enable nodejs:18 ruby:3.1 && \
dnf -y module enable nodejs:20 ruby:3.3 && \
dnf install -y \
file \
lsof \
Expand All @@ -23,6 +23,7 @@ RUN dnf -y update && \
patch \
lua-posix \
rsync \
xz \
ondemand-gems \
ondemand-runtime \
ondemand-build \
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile.dev
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ RUN cp /etc/yum.repos.d/ondemand-web.repo /etc/yum.repos.d/ondemand-nightly-web.
RUN dnf -y update && \
dnf install -y dnf-utils && \
dnf config-manager --set-enabled powertools && \
dnf -y module enable nodejs:18 ruby:3.1 && \
dnf -y module enable nodejs:20 ruby:3.3 && \
dnf install -y epel-release && \
dnf install -y ondemand ondemand-dex && \
dnf install -y gcc gcc-c++ libyaml-devel nc && \
Expand Down
4 changes: 2 additions & 2 deletions apps/dashboard/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ GEM
railties (>= 6.0.0)
local_time (1.0.3)
coffee-rails
logger (1.6.4)
logger (1.6.5)
lograge (0.14.0)
actionpack (>= 4)
activesupport (>= 4)
Expand All @@ -159,7 +159,7 @@ GEM
mime-types (3.6.0)
logger
mime-types-data (~> 3.2015)
mime-types-data (3.2024.1203)
mime-types-data (3.2025.0107)
mini_mime (1.1.5)
mini_portile2 (2.8.8)
minitest (5.25.4)
Expand Down
31 changes: 31 additions & 0 deletions apps/dashboard/app/controllers/widgets_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# frozen_string_literal: true

# The Controller to render widget templates without any layout furniture
class WidgetsController < ApplicationController

def show
widget_path = File.join('/widgets', params[:widget_path])

unless valid_path?(widget_path)
render plain: "400 Bad Request. Invalid widget path: #{widget_path}", status: :bad_request
return
end


widget_exists = lookup_context.exists?(widget_path, [], true)
unless widget_exists
render plain: "404 Widget not found: #{widget_path}", status: :not_found
return
end

render partial: widget_path, layout: false
end

private

# Checks if the widget path contains only allowed characters
def valid_path?(widget_path)
widget_path.match?(/\A[a-zA-Z0-9_\-\/]+\z/)
end
end

23 changes: 15 additions & 8 deletions apps/dashboard/app/javascript/icon_picker.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,22 @@ function searchIcons(event) {
const uniqueSearchCharacters = new Set(searchString.split(''));
const searchCharacters = [...uniqueSearchCharacters].filter((char) => indexKeys.includes(char));

searchCharacters.forEach(char => {
const indices = invertedIndex[char]; // Get indices for the character in the inverted index
indices.forEach(index => {
const iconStr = ALL_ICONS[index].toLowerCase();
if (iconStr.includes(searchString)) {
resultIndices.add(index);
}
// Account for boundary condition where the search string is empty
if(searchString.length === 0) {
for(let i = 0; i < ALL_ICONS.length; i++) {
resultIndices.add(i);
}
} else {
searchCharacters.forEach(char => {
const indices = invertedIndex[char]; // Get indices for the character in the inverted index
indices.forEach(index => {
const iconStr = ALL_ICONS[index].toLowerCase();
if (iconStr.includes(searchString)) {
resultIndices.add(index);
}
});
});
});
}

ALL_ICONS.forEach((name, idx) => {
const ele = $(`#${iconId(name)}`)[0];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ function makeTable(element) {
}

function getPathSelectorOptions(element) {
options = {};
const options = {};

options.filesPath = element.dataset['filesPath'];
options.initialDirectory = element.dataset['initialDirectory'];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { alert } from '../alert';
import { hide, show } from "../utils";

export class PathSelectorTable {
_table = null;
Expand Down Expand Up @@ -87,7 +88,7 @@ export class PathSelectorTable {
async reloadTable(url) {
try {
$(this.tableWrapper()).hide();
$(this).closest('.loading-icon').show();
show(`${this.tableId}_spinner`);
const response = await fetch(url, { headers: { 'Accept': 'application/json' }, cache: 'no-store' });
const data = await this.dataFromJsonResponse(response);
$(`#${this.breadcrumbId}`).html(data.path_selector_breadcrumbs_html);
Expand All @@ -107,7 +108,7 @@ export class PathSelectorTable {
}

resetTable() {
$(this).closest(".loading-icon").hide();
hide(`${this.tableId}_spinner`);
$(this.tableWrapper()).show();
$('#forbidden-warning').addClass('d-none');
}
Expand Down
20 changes: 20 additions & 0 deletions apps/dashboard/app/javascript/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,23 @@ export function reportErrorForAnalytics(path, error) {
// Fire and Forget
fetch(analyticsUrl);
}

// helper method to hide an element. Note that jQuery's hide()
// changes the inline style which may not do anything if the element
// already has a bootstrap display class like d-flex.
export function hide(id) {
const ele = document.getElementById(id);
if(ele !== null) {
ele.classList.add('d-none');
}
}

// helper method to show an element. Note that jQuery's show()
// changes the inline style which may not do anything if the element
// already has a bootstrap display class like d-flex.
export function show(id) {
const ele = document.getElementById(id);
if(ele !== null) {
ele.classList.remove('d-none');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
<%- if session.starting? || session.running? -%>
<span class="badge <%= badge_class %> rounded-pill"><%= pluralize(num_nodes, "node") %></span>
<span class="card-text"> | </span>
<span class="badge <%= badge_class %> rounded-pill"><%= pluralize(num_nodes, "core") %></span>
<span class="badge <%= badge_class %> rounded-pill"><%= pluralize(num_cores, "core") %></span>
<span class="card-text"> | </span>
<%- end -%>
<%= status(session) %>
Expand Down
2 changes: 1 addition & 1 deletion apps/dashboard/app/views/projects/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
<div class='card-body'>
<div class="col">
<div class="field">
<%= javascript_include_tag 'icon_picker', nonce: true %>
<%= javascript_include_tag('icon_picker', nonce: true, type: 'module') %>
<%= form.text_field :icon, placeholder: "cog", id: "product_icon_select", value: @project.icon_class %>
<% if @project.icon =~ /(fa[bsrl]?):\/\/(.*)/ %>
<% icon = $2; style = $1 %>
Expand Down
2 changes: 1 addition & 1 deletion apps/dashboard/app/views/projects/new.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<%= javascript_include_tag 'projects_new', nonce: true %>
<%= javascript_include_tag('projects_new', nonce: true, type: 'module') %>

<div class='page-header text-center'>
<h1>New Project</h1>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
<%= t('dashboard.batch_connect_sessions_path_selector_forbidden_error') %>
</div>

<div class="d-flex justify-content-center">
<div class="d-flex justify-content-center" id="<%= table_id %>_spinner">
<div class="loading-icon spinner-border rem-5" role="status">
<span class="sr-only">Loading...</span>
</div>
Expand Down
1 change: 1 addition & 0 deletions apps/dashboard/config/configuration_singleton.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ def boolean_configs
:upload_enabled => true,
:download_enabled => true,
:project_size_enabled => true,
:widget_partials_enabled => false,
}.freeze
end

Expand Down
7 changes: 7 additions & 0 deletions apps/dashboard/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,13 @@

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

# 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

# Support ticket routes
if Configuration.support_ticket_enabled?
get '/support', to: 'support_ticket#new'
Expand Down
45 changes: 45 additions & 0 deletions apps/dashboard/test/controllers/widgets_controller_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
require "test_helper"

class WidgetsControllerTest < ActiveSupport::TestCase

def setup
@controller = WidgetsController.new
end

test 'valid_path? validates widget paths' do
refute @controller.send(:valid_path?, '/test!')
refute @controller.send(:valid_path?, '/test/../../outside_dir')
refute @controller.send(:valid_path?, '@user:pwd/dir')

assert @controller.send(:valid_path?, 'test')
assert @controller.send(:valid_path?, '/test')
assert @controller.send(:valid_path?, '/test/path/widget')
assert @controller.send(:valid_path?, '/test_path/widget')
assert @controller.send(:valid_path?, '/test-path/widget_under/name')
end

test 'show should return HTTP 400 when invalid widget path is used' do
@params = ActionController::Parameters.new({ widget_path: '!!invalid' })
@controller.stubs(:params).returns(@params)
@controller.expects(:render).with(plain: '400 Bad Request. Invalid widget path: /widgets/!!invalid', status: :bad_request)

@controller.show
end

test 'show should return HTTP 404 when valid widget path is not found in the system' do
@params = ActionController::Parameters.new({ widget_path: '/valid/path' })
@controller.stubs(:params).returns(@params)
@controller.expects(:render).with(plain: '404 Widget not found: /widgets/valid/path', status: :not_found)

@controller.show
end

test 'show should render widget when valid widget path is found in the system' do
@params = ActionController::Parameters.new({ widget_path: '/valid/path' })
@controller.stubs(:params).returns(@params)
@controller.lookup_context.stubs(:exists?).returns(true)
@controller.expects(:render).with(partial: '/widgets/valid/path', layout: false)

@controller.show
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<h3>test response from widget partial</h3>
30 changes: 30 additions & 0 deletions apps/dashboard/test/integration/widgets_partial_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
require 'html_helper'
require 'test_helper'

class WidgetsPartialTest < ActionDispatch::IntegrationTest

def setup
Configuration.stubs(:widget_partials_enabled?).returns(true)
Rails.application.reload_routes!
end

test 'should not load widgets partial route when widget_partials_enabled? is false' do
Configuration.stubs(:widget_partials_enabled?).returns(false)
Rails.application.reload_routes!

refute respond_to?(:widgets_url)
end

test 'should render widget partial without any layout furniture' do
get widgets_url('widgets_partial_test')

assert_response :ok
assert_equal '<h3>test response from widget partial</h3>', @response.body
end

test 'should render return 404 response when widget is missing' do
get widgets_url('missing_widget')

assert_response :not_found
end
end
5 changes: 3 additions & 2 deletions apps/dashboard/test/system/batch_connect_widgets_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,13 @@ def make_bc_app(dir, form)
visit new_batch_connect_session_context_url('sys/app')

click_on 'Select Path'
sleep 0.5

table_id = "batch_connect_session_context_path_path_selector_table"
shown_dirs_and_files = find_all("##{table_id} tr td").map { |td| td["innerHTML"] }

assert shown_dirs_and_files.any? { |file| file.match("test.py") }
refute shown_dirs_and_files.any? { |file| file.match("test.rb") }
assert(shown_dirs_and_files.any? { |file| file.match("test.py") })
refute(shown_dirs_and_files.any? { |file| file.match("test.rb") })
end
end

Expand Down
11 changes: 9 additions & 2 deletions apps/dashboard/test/system/project_manager_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,18 @@ def add_auto_environment_variable(project_id, launcher_id, save: true)
end

test 'searching icons works' do
# TODO
visit(new_project_path)
find('#product_icon_select').set('')
find('#product_icon_select').set('cog')
icons = find('#icon_picker_list').all('i')
assert_equal(4, icons.size)
end

test 'all icons show after clearing input field' do
# TODO
visit(new_project_path)
find('#product_icon_select').set('')
icons = find('#icon_picker_list').all('i')
assert_equal(990, icons.size)
end

test 'creating and showing launchers' do
Expand Down
Loading

0 comments on commit 76da4f1

Please sign in to comment.