Skip to content

Action scripts#41

Draft
Gofannon wants to merge 23 commits into
YunoHost-Apps:testingfrom
Gofannon:action_scripts
Draft

Action scripts#41
Gofannon wants to merge 23 commits into
YunoHost-Apps:testingfrom
Gofannon:action_scripts

Conversation

@Gofannon

@Gofannon Gofannon commented Sep 28, 2018

Copy link
Copy Markdown
Collaborator

Problem

  • Add "action scripts" => commited, should work
  • Add "config panel" => Not in the PR, does not run for now

Work in progress. Not ready to merge ! Is used to show progress and let the Apps team work is ongoing

action scripts

To test it:

yunohost app action list dokuwiki

yunohost app action run dokuwiki public_private -a is_public=1 --debug
yunohost app action run dokuwiki public_private -a is_public=0 --debug

yunohost app action run dokuwiki internal_users -a is_internal_users=1 --debug
yunohost app action run dokuwiki internal_users -a is_internal_users=0 --debug

Followed examples

Solution

PR Status

  • Code finished.
  • Tested with Package_check.
  • Fix or enhancement tested.
  • Upgrade from last version tested.
  • Can be reviewed and tested.

Validation


Minor decision

  • Upgrade previous version :
  • Code review : Maniack C
  • Approval (LGTM) : Maniack C
  • Approval (LGTM) : Kay0u
  • CI succeeded : Build Status
    When the PR is marked as ready to merge, you have to wait for 3 days before really merging it.

Comment thread scripts/upgrade Outdated
final_path=$(ynh_app_setting_get $app final_path)

auth_backend=$(ynh_app_setting_get $app auth_backend)
#is_internal_users=$(ynh_app_setting_get $app is_internal_users)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You should probably uncomment is_internal_users, as you check it after.

Comment thread scripts/upgrade
if [ -z "$is_internal_users" ]; then
ynh_app_setting_set $app is_internal_users 0
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

A test is needed as well for auth_backend

Comment thread scripts/actions/internal_users Outdated
#=================================================

# Get authentication backend and set as lowercase
#auth_backend=${YNH_ACTION_IS_INTERNAL_USERS,,}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This line should be removed I guess.

Comment thread scripts/actions/internal_users Outdated
# Plugin "authchained" needs to be installed for this script to run
if [ ! -d $final_path/lib/plugins/authchained ]; then
ynh_die "Plugin 'authchained' is not installed and must be installed beforehand." 1
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Small "typo", but those 2 previous check should be in "# CHECK IF ARGUMENTS AND REQUIREMENTS ARE CORRECT"

Comment thread scripts/actions/internal_users Outdated


# Regen ssowat configuration
yunohost app ssowatconf

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unneeded I think

Comment thread scripts/actions/internal_users Outdated
# RELOAD NGINX
#=================================================

systemctl reload nginx

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Don't think you need that either.

@maniackcrudelis
maniackcrudelis requested a review from a team March 10, 2019 14:09
@maniackcrudelis

Copy link
Copy Markdown

Not tested, but looks good.

@kay0u

kay0u commented Apr 17, 2019

Copy link
Copy Markdown
Member

bump

Gofannon added 4 commits May 12, 2019 15:55
Add this setting even if not use by default. Only usefull with "internal_users" script.
Harmless and makes the maintenance easier
@Gofannon

Copy link
Copy Markdown
Collaborator Author

Can be reviewed and tested.

What has been tested on a VM

  1. Install a new "master branch" dokuwiki
  2. Upgrade using the PR
  3. Upgrade again
  4. Multiple combination of the following:
yunohost app action list dokuwiki

yunohost app action run dokuwiki public_private -a is_public=1 --debug
yunohost app action run dokuwiki public_private -a is_public=0 --debug

yunohost app action run dokuwiki internal_users -a is_internal_users=1 --debug
yunohost app action run dokuwiki internal_users -a is_internal_users=0 --debug

@Gofannon

Copy link
Copy Markdown
Collaborator Author

merge conflicts solved

@Gofannon

Copy link
Copy Markdown
Collaborator Author

bump

kay0u
kay0u previously approved these changes May 23, 2019

@kay0u kay0u left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@Gofannon

Gofannon commented Jul 5, 2019

Copy link
Copy Markdown
Collaborator Author

Following the release of YunoHost 3.6, shall I use TOML instead of JSON?

@maniackcrudelis

Copy link
Copy Markdown

It's supposed to work with json and toml. But, it's clearly easier to read in toml. So would be better for long term support to write directly in toml

@tituspijean
tituspijean marked this pull request as draft September 14, 2022 16:21
@ericgaspar

Copy link
Copy Markdown
Member

!testme

@yunohost-bot

Copy link
Copy Markdown

Living in the future, are we?
Test Badge

@yunohost-bot

Copy link
Copy Markdown

✌️
Test Badge

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants