Skip to content

Commit

Permalink
Maintenance: Remove feature flag for mobile front end and make Redis …
Browse files Browse the repository at this point in the history
…a regular dependency.
  • Loading branch information
mgruner committed Apr 18, 2023
1 parent 51d08a1 commit 52306c6
Show file tree
Hide file tree
Showing 20 changed files with 60 additions and 117 deletions.
2 changes: 0 additions & 2 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ jobs:
RAILS_ENV: test
Z_LOCALES: "en-us:de-de"
REDIS_URL: redis://redis:6379
# Temporary switch to enable the mobile front end for testing.
ENABLE_EXPERIMENTAL_MOBILE_FRONTEND: 'true'
# Compile assets only once.
CI_SKIP_ASSETS_PRECOMPILE: 'true'
# Avoid unnecessary DB resets.
Expand Down
2 changes: 0 additions & 2 deletions .gitlab/ci/__includes__/variables.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ variables:
ES_INDEX: 'estest.ci'
ELASTICSEARCH_TAG: '7.16'

# Temporary switch to enable the mobile front end for testing.
ENABLE_EXPERIMENTAL_MOBILE_FRONTEND: 'true'
# Compile assets only once.
CI_SKIP_ASSETS_PRECOMPILE: 'true'
# Avoid unnecessary DB resets.
Expand Down
4 changes: 2 additions & 2 deletions .gitlab/ci/test/migration-from-mysql-to-postgresql.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
allow_failure: true
- when: on_success
variables:
# Turn off not not require redis for MySQL/MariaDB.
ENABLE_EXPERIMENTAL_MOBILE_FRONTEND: ''
ENFORCE_DB_SERVICE: mysql
script:
- !reference [.scripts, configure_environment]
Expand All @@ -35,10 +33,12 @@ migration:database:mysql_to_postgresql:
services:
- !reference [.services, mysql]
- !reference [.services, postgresql]
- !reference [.services, redis]

migration:database:mariadb_to_postgresql:
extends:
- .template_migration_from_mysql_to_postgresql
services:
- !reference [.services, mariadb]
- !reference [.services, postgresql]
- !reference [.services, redis]
8 changes: 5 additions & 3 deletions .gitlab/ci/test/migration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@
when: manual
allow_failure: true
- when: on_success
variables:
# Turn off not not require redis for MySQL/MariaDB.
ENABLE_EXPERIMENTAL_MOBILE_FRONTEND: ''
before_script: []
script:
# First, checkout stable and set it up.
Expand All @@ -27,6 +24,8 @@
- git checkout $CI_COMMIT_SHA
- rvm use 3.1.3
- !reference [.scripts, bundle_install]
# Force redis usage, even if it was disabled by the initial configure_environment script of stable.
- export REDIS_URL=redis://redis
- bundle exec rails db:migrate
- bundle exec rspec --profile 10 spec/db

Expand All @@ -35,15 +34,18 @@ rspec:migration:postgresql:
- .template_migration
services:
- !reference [.services, postgresql]
- !reference [.services, redis]

rspec:migration:mysql:
extends:
- .template_migration
services:
- !reference [.services, mysql]
- !reference [.services, redis]

rspec:migration:mariadb:
extends:
- .template_migration
services:
- !reference [.services, mariadb]
- !reference [.services, redis]
3 changes: 1 addition & 2 deletions .gitlab/configure_environment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def self.configure_database # rubocop:disable Metrics/AbcSize

def self.configure_redis
has_redis = network_host_exists?('redis')
needs_redis = !%w[1 true].include?(ENV['ZAMMAD_SAFE_MODE']) && ENV['ENABLE_EXPERIMENTAL_MOBILE_FRONTEND'] == 'true' # rubocop:disable Rails/NegateInclude
needs_redis = !%w[1 true].include?(ENV['ZAMMAD_SAFE_MODE']) # rubocop:disable Rails/NegateInclude

if needs_redis && !has_redis
raise 'Redis was not found, but is required for ActionCable.'
Expand Down Expand Up @@ -135,7 +135,6 @@ def self.write_env_file
end

def self.run
puts 'ENABLING THE NEW EXPERIMENTAL MOBILE FRONTEND.' if ENV['ENABLE_EXPERIMENTAL_MOBILE_FRONTEND'] == 'true'
configure_database
configure_redis
configure_memcached
Expand Down
12 changes: 12 additions & 0 deletions .pkgr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ targets:
- epel-release
- imlib2
- shared-mime-info
- redis
build_dependencies:
- https://download.fedoraproject.org/pub/epel/7/x86_64/Packages/i/imlib2-1.4.9-8.el7.x86_64.rpm
- https://download.fedoraproject.org/pub/epel/7/x86_64/Packages/i/imlib2-devel-1.4.9-8.el7.x86_64.rpm
Expand All @@ -28,6 +29,7 @@ targets:
- epel-release
- imlib2
- shared-mime-info
- redis
build_dependencies:
- https://download.fedoraproject.org/pub/epel/8/Everything/x86_64/Packages/i/imlib2-1.4.9-8.el8.x86_64.rpm
- https://download.fedoraproject.org/pub/epel/8/Everything/x86_64/Packages/i/imlib2-devel-1.4.9-8.el8.x86_64.rpm
Expand All @@ -40,6 +42,7 @@ targets:
- postgresql|mariadb-server
- libimlib2
- shared-mime-info
- redis
build_dependencies:
- libimlib2
- libimlib2-dev
Expand All @@ -52,6 +55,7 @@ targets:
- postgresql|mariadb-server
- libimlib2
- shared-mime-info
- redis
build_dependencies:
- libimlib2
- libimlib2-dev
Expand All @@ -64,6 +68,7 @@ targets:
- postgresql|mariadb-server
- libimlib2
- shared-mime-info
- redis
build_dependencies:
- libimlib2
- libimlib2-dev
Expand All @@ -76,6 +81,7 @@ targets:
- postgresql|mysql-server|mariadb-server
- libimlib2
- shared-mime-info
- redis
build_dependencies:
- libimlib2
- libimlib2-dev
Expand All @@ -88,6 +94,7 @@ targets:
- postgresql|mysql-server|mariadb-server
- libimlib2
- shared-mime-info
- redis
build_dependencies:
- libimlib2
- libimlib2-dev
Expand All @@ -100,6 +107,7 @@ targets:
- postgresql|mysql-server|mariadb-server
- libimlib2
- shared-mime-info
- redis
build_dependencies:
- libimlib2
- libimlib2-dev
Expand All @@ -112,6 +120,7 @@ targets:
- postgresql|mysql-server|mariadb-server
- libimlib2
- shared-mime-info
- redis
build_dependencies:
- libimlib2
- libimlib2-dev
Expand All @@ -124,6 +133,7 @@ targets:
- postgresql-server
- libImlib2-1
- imlib2
- redis
build_dependencies:
# Add packages required for build that are not in the official SLES repo.
# Direct URLs must be used since we cannot add repos on packager.io
Expand All @@ -142,6 +152,7 @@ targets:
- libImlib2-1
- imlib2
- shared-mime-info
- redis
build_dependencies:
# Add packages required for build that are not in the official SLES repo.
# Direct URLs must be used since we cannot add repos on packager.io
Expand All @@ -163,5 +174,6 @@ env:
- ZAMMAD_WEBSOCKET_PORT=6042
services:
- postgres:13
- redis
before_install: contrib/packager.io/preinstall.sh
after_install: contrib/packager.io/postinstall.sh
4 changes: 2 additions & 2 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ gem 'thor'
# core - websocket
gem 'em-websocket'
gem 'eventmachine'
gem 'hiredis', require: false
gem 'hiredis'
# version restriction from actioncable-6.1.6.1/lib/action_cable/subscription_adapter/redis.rb
# - check after rails update
gem 'redis', '>= 3', '< 5', require: false
gem 'redis', '>= 3', '< 5'

# core - password security
gem 'argon2'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
class App.MobileDetection
@isMobile: ->
# TODO: Remove `mobile_frontend_enabled` check when this switch is not needed any more.
App.Config.get('mobile_frontend_enabled') and isMobile()
isMobile()

@isForcingDesktopView: ->
# TODO: Remove `mobile_frontend_enabled` check when this switch is not needed any more.
App.Config.get('mobile_frontend_enabled') and App.LocalStorage.get('forceDesktopApp', false)
App.LocalStorage.get('forceDesktopApp', false)

@clearForceDesktopApp: ->
if App.LocalStorage.get('forceDesktopApp', false)
Expand All @@ -20,9 +18,6 @@ class App.MobileDetection
window.location.href = target

@redirectToMobile: =>
# TODO: Remove this when the mobile frontend switch is not needed any more.
return if not App.Config.get('mobile_frontend_enabled')

@clearForceDesktopApp()
@navigateToMobile()

Expand Down Expand Up @@ -54,7 +49,6 @@ class App.MobileDetectionPlugin extends App.Controller
App.Config.set('mobile_detection', App.MobileDetectionPlugin, 'Plugins')

if App.MobileDetection.isMobile() or App.LocalStorage.get('forceDesktopApp', false)
# TODO: Remove `mobile_frontend_enabled` check when this switch is not needed any more.
App.Config.set('Mobile',
{
prio: 1500,
Expand All @@ -64,6 +58,5 @@ if App.MobileDetection.isMobile() or App.LocalStorage.get('forceDesktopApp', fal
target: '#',
onclick: true,
key: 'MobileDetection',
setting: ['mobile_frontend_enabled']
}
, 'NavBarRight')
4 changes: 0 additions & 4 deletions app/controllers/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -279,10 +279,6 @@ def config_frontend
config['session_id'] = session.id.public_id
end

# Temporary Hack: include the info about mobile frontend switch in the frontend config.
# TODO: Remove when this switch is not needed any more.
config['mobile_frontend_enabled'] = ENV['ENABLE_EXPERIMENTAL_MOBILE_FRONTEND'] == 'true'

config
end

Expand Down
1 change: 0 additions & 1 deletion app/graphql/gql/subscriptions/base_subscription.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ def self.broadcastable?
def self.trigger(object, arguments: {}, scope: nil)

return if Setting.get('import_mode') || Zammad::SafeMode.enabled?
return if ENV['ENABLE_EXPERIMENTAL_MOBILE_FRONTEND'] != 'true'

::Gql::ZammadSchema.subscriptions.trigger(
graphql_field_name,
Expand Down
7 changes: 0 additions & 7 deletions config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,6 @@
require_relative 'issue_2656_workaround_for_rails_issue_33600'
require_relative '../lib/zammad/safe_mode'

# Temporary Hack: skip vite build if ENABLE_EXPERIMENTAL_MOBILE_FRONTEND is not set.
# This must be called before ViteRuby is loaded by Bundler.
# TODO: Remove when this switch is not needed any more.
if ENV['ENABLE_EXPERIMENTAL_MOBILE_FRONTEND'] != 'true'
ENV['VITE_RUBY_SKIP_ASSETS_PRECOMPILE_EXTENSION'] = 'true'
end

# DO NOT REMOVE THIS LINE - see issue #2037
Bundler.setup

Expand Down
63 changes: 29 additions & 34 deletions config/initializers/action_cable_preferences.rb
Original file line number Diff line number Diff line change
@@ -1,42 +1,37 @@
# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/

if ENV['ENABLE_EXPERIMENTAL_MOBILE_FRONTEND'] == 'true'
require 'redis'
require 'hiredis'
# If REDIS_URL is not set, fall back to default port / localhost, to ease configuration
# for simple installations.
redis_url = ENV['REDIS_URL'].presence || 'redis://localhost:6379'

# If REDIS_URL is not set, fall back to default port / localhost, to ease configuration
# for simple installations.
redis_url = ENV['REDIS_URL'].presence || 'redis://localhost:6379'

Rails.application.config.action_cable.cable = {
adapter: :redis,
driver: :hiredis,
url: redis_url,
channel_prefix: "zammad_#{Rails.env}",
}
begin
Redis.new(driver: :hiredis, url: redis_url).ping
Rails.logger.info { "ActionCable is using the redis instance at #{redis_url}." }
rescue Redis::CannotConnectError => e
warn "There was an error trying to connect to Redis via #{redis_url}."
if ENV['REDIS_URL'].present?
warn 'Please make sure Redis is available.'
else
warn 'Please provide a Redis instance at localhost:6379 or set REDIS_URL to point to a different location.'
end
warn e.inspect
Zammad::SafeMode.continue_or_exit!
Rails.application.config.action_cable.cable = {
adapter: :redis,
driver: :hiredis,
url: redis_url,
channel_prefix: "zammad_#{Rails.env}",
}
begin
Redis.new(driver: :hiredis, url: redis_url).ping
Rails.logger.info { "ActionCable is using the redis instance at #{redis_url}." }
rescue Redis::CannotConnectError => e
warn "There was an error trying to connect to Redis via #{redis_url}."
if ENV['REDIS_URL'].present?
warn 'Please make sure Redis is available.'
else
warn 'Please provide a Redis instance at localhost:6379 or set REDIS_URL to point to a different location.'
end
warn e.inspect
Zammad::SafeMode.continue_or_exit!
end

if Rails.env.production?
Rails.application.reloader.to_prepare do
begin
request_origins = ['http://localhost:3000']
request_origins << "#{Setting.get('http_type')}://#{Setting.get('fqdn')}"
Rails.application.config.action_cable.allowed_request_origins = request_origins
rescue ActiveRecord::NoDatabaseError
Rails.logger.debug("Database doesn't exist. Skipping allowed_request_origins configuration.")
end
if Rails.env.production?
Rails.application.reloader.to_prepare do
begin
request_origins = ['http://localhost:3000']
request_origins << "#{Setting.get('http_type')}://#{Setting.get('fqdn')}"
Rails.application.config.action_cable.allowed_request_origins = request_origins
rescue ActiveRecord::NoDatabaseError, ActiveRecord::StatementInvalid
Rails.logger.debug("Database doesn't exist. Skipping allowed_request_origins configuration.")
end
end
end
9 changes: 2 additions & 7 deletions config/routes/graphql.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/

# Temporary Hack: only process trigger events if ActionCable is enabled.
# TODO: Remove when this switch is not needed any more.

if ENV['ENABLE_EXPERIMENTAL_MOBILE_FRONTEND'] == 'true'
Zammad::Application.routes.draw do
match '/graphql', to: 'graphql#execute', via: %i[options post]
end
Zammad::Application.routes.draw do
match '/graphql', to: 'graphql#execute', via: %i[options post]
end
15 changes: 5 additions & 10 deletions config/routes/mobile.rb
Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/

# Temporary Hack: only process trigger events if ActionCable is enabled.
# TODO: Remove when this switch is not needed any more.

if ENV['ENABLE_EXPERIMENTAL_MOBILE_FRONTEND'] == 'true'
Zammad::Application.routes.draw do
get '/mobile', to: 'mobile#index'
get '/mobile/sw.js', to: 'mobile#service_worker'
get '/mobile/manifest.webmanifest', to: 'mobile#manifest'
get '/mobile/*path', to: 'mobile#index'
end
Zammad::Application.routes.draw do
get '/mobile', to: 'mobile#index'
get '/mobile/sw.js', to: 'mobile#service_worker'
get '/mobile/manifest.webmanifest', to: 'mobile#manifest'
get '/mobile/*path', to: 'mobile#index'
end
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,6 @@ sudo systemctl restart elasticsearch.service

## Redis

If you would like to develop something for the new mobile front end, you need

- to set `ENABLE_EXPERIMENTAL_MOBILE_FRONTEND` to `true`
- and install a Redis server on your machine.

For macOS:

```screen
Expand Down
Loading

0 comments on commit 52306c6

Please sign in to comment.