Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#280 closed defect (fixed)

Update the installation process

Reported by: markoheijnen Owned by: markoheijnen
Milestone: 1.0 Priority: blocker
Version: Component: interface
Keywords: gsoc has-patch Cc: daisuke@…


Currently the installation of GlotPress has a lame user experience like setting the default username to 'admin' with a password of 'a'. Also it doesn't generate the .htaccess if needed. The workflow should more look like how WordPress does it.

Attachments (6)

htaccess-add-install.patch (2.4 KB) - added by Secretmapper 4 years ago.
htaccess-add-install-own-method.patch (2.7 KB) - added by Secretmapper 4 years ago.
htaccess-add-install-clean.patch (4.5 KB) - added by Secretmapper 4 years ago.
Cleaned up the file
install-form.diff (9.9 KB) - added by Secretmapper 4 years ago.
install.2.patch (10.3 KB) - added by Secretmapper 4 years ago.
install.3.diff (10.3 KB) - added by Secretmapper 4 years ago.

Download all attachments as: .zip

Change History (28)

#2 @Secretmapper
4 years ago

I already have this done except for one thing.

In the string made to add to htaccess made by the install template it is using a '$path' variable to append to RewriteBase.

		# BEGIN GlotPress
		&lt;IfModule mod_rewrite.c&gt;
		RewriteEngine On
		RewriteBase <?php echo $path . "\n"; ?>
		RewriteCond %{REQUEST_FILENAME} !-f
		RewriteCond %{REQUEST_FILENAME} !-d
		RewriteRule . <?php echo $path; ?>index.php [L]
		# END GlotPress

I can't seem to access the $path inside the actual install script (install.php), so ultimately the rewrite rule is failing. Is it being added by wordpress dynamically? Anyway, I'll take a look into it.

#3 @markoheijnen
4 years ago

It's not defined in the template but in /install.php. So that should be fine.

#4 follow-up: @Secretmapper
4 years ago

Chalk it up to working so late at night :)

#5 in reply to: ↑ 4 @yoavf
4 years ago

Hi Secretmapper,

Thanks for the patch!

Can you take a look at the WordPress coding standards and update it accordingly?

#6 @Secretmapper
4 years ago

Hey yoavf,

I've moved the htaccess add script to its own method in the install class, and also fixed the code to follow the coding standards (e.g. yoda conditions, etc.)

Tried my best to follow the coding standards, but if you see a mistake feel free to nitpick!

P.S. Had a ton of paperwork as college ended, so update was a bit late.

#7 @markoheijnen
4 years ago

So this patch still doesn't apply the coding standards like:

  • Indenting with tabs and not spaces
  • Add more spaces like if( will become if (

Codewise I hate the duplication of the .htaccess code. I believe we could make a function out of that one to.

4 years ago

Cleaned up the file

#8 @Secretmapper
4 years ago

Third time's the charm :) Turns out my IDE was converting tabs to spaces.

I've also removed the duplicated htaccess string, so its only defined in the install script.

#9 @markoheijnen
4 years ago

In 871:

Generate the .htaccess file when possible.

Props Secretmapper for the initial patch.
See #280

#10 @markoheijnen
4 years ago

The indentation was still spaces so I updated that in my commit. I also changed the code a bit to check if mod rewrite is enabled and the response of gp_set_htaccess() is now always a boolean. I also added the empty( $errors ) back to install.php.

Please check the commit and see the changes I made.

#11 @Secretmapper
4 years ago

Install form to mirror WP Install form.

Todo: take into account existing table (WP database link)

#12 @markoheijnen
4 years ago

You made some good progress. Next time also test the patch ;) It doesn't work due the fact you first create a user before creating the database gp_install().

A few other notes:

  • User creating probably should stick in gp_create_initial_contents(). Now it exists in multiple places.
  • Brackets around __() aren't needed in install.php.
  • I think we should put guess_uri() inside gp-includes/url.php and have it as the fallback value in gp_url_base_root().
  • Unsure why we had <pre> for the error messages. I would love to delete that.
  • $action gets wrong value when you successfully installed GlotPress. It should be install and not upgrade.

When CUSTOM_USER_TABLE is defined we can fallback to old behaviour.

Last edited 4 years ago by markoheijnen (previous) (diff)

#13 @Secretmapper
4 years ago

Latest patch addresses @marko's points.

<pre> is for the SQL errors

#14 @markoheijnen
4 years ago

Two last things and then it's fine for me to commit this:

  • When CUSTOM_USER_TABLE is defined there should not be any user created.
  • We don't have an action installed. Only install and upgrade. The logic from gp_breadcrumb() will always fallback to upgrade then. We could switch that but I don't see any benefit for it.

#15 @Secretmapper
4 years ago

We don't have an action installed. Only install and upgrade. The logic from gp_breadcrumb() will always fallback to upgrade then. We could switch that but I don't see any benefit for it.

It was a sort of sentinel value. The 'installed' action will hide the install form, while showing install in the breadcrumb (I switched the breadcrumb to fallback to install). In hindsight it is indeed a bit hard to read, maybe I should just add a new installing variable?

#16 @markoheijnen
4 years ago

  • Keywords gsoc added

I was checking the latest patch. Styling can be fix later but the following still needs to be fixed:

  • No need to have $_GET['step'] == 2 since we only have one
  • No need for $error while we just can check for $errors.

#17 @extendwings
4 years ago

  • Cc daisuke@… added

In r871, gp_mod_rewrite_rules() returns .htaccess with unwanted semicolon. I think we must get rid of it.
Here is a my Git commit and diff file URL.

#18 @markoheijnen
4 years ago

In 949:

Remove unwanted semicolon when generating .htaccess file. Props extendwings. See #280.

#19 @markoheijnen
4 years ago

  • Keywords has-patch added

#20 @markoheijnen
4 years ago

  • Owner set to markoheijnen
  • Resolution set to fixed
  • Status changed from new to closed

In 953:

Complete overhaul of the installation process. You can now fill in your username, password and email address.

Props Secretmapper
Fixes #280

#21 @markoheijnen
3 years ago

In 963:

Hide login link during installation. See #280

#22 @markoheijnen
3 years ago

In 1089:

Fix check when mod_rewrite isn't loaded to not set the .htaccess file. This due to debugging in the beginning.

See #280

Note: See TracTickets for help on using tickets.