elementor / wp2static-addon-s3

S3 deployment Add-on for WP2Static
The Unlicense
35 stars 23 forks source link

add htaccess redirects #20

Closed maulikkanani closed 4 years ago

leonstafford commented 4 years ago

Hi @maulikkanani - thanks for submitting this! Sorry for my delay in checking it until now.

Could you please rebase this on latest master and drop the error_log lines? They can be WsLog::l() instead if needed to be logged.

I'll need some more time to understand the actual implementation. Please feel free to add more info here about what it does.

Cheers,

Leon

leonstafford commented 4 years ago

Hi @maulikkanani - you may get some ideas from this recently merged PR: https://github.com/WP2Static/wp2static-addon-s3/pull/29

maulikkanani commented 4 years ago

it seems like Master repo has been updated so I guess I need to resolve conflict then need to commit it again.

Here is detail for what I have developed.

I have created "Create page redirects found in .htaccess" checkbox for .htacces RewriteRule redirect on the S3 bucket. If the checkbox is selected then it will reader .htaccess and find 301 redirects in it and set same redirection on s3bucket as well.

Would you please refer: For Checkbox: https://www.screenpresso.com/=bAutg For .htacces RewriteRule : https://www.screenpresso.com/=QsUwc For S3 Bucket: https://www.screenpresso.com/=1Vlrb

Let me know if you need further details.

john-shaffer commented 4 years ago

29 handles deploying redirects from the wp2static_list_redirects filter. That's all that the S3 addon needs to do. The detection of redirects from other sources is out of scope for this addon.

It could go into WP2Static core, but Leon wants to keep the options there to a minimum, so it probably won't make it in. I think there is a plan for an Advanced Crawling Options Addon or something like that, so that would probably be the place for this.

leonstafford commented 4 years ago

Thanks for updating @maulikkanani.

I agree with @john-shaffer that the .htaccess doesn't need to be within this S3 repo. But also not in the core plugin, If still needed for your use case now that the wp2static_list_redirects filter is available, then I'd prefer it to be in it's own Add-on, which users can then use with any deployment option. This can be your own add-on, which I'm happy to promote with any 3rd party add-ons.

I know of some users who have very complex htaccess setups and would like to translate more than just rewrite rules to the deployed site host, I can imagine that would get tricky and it's unlikely I'm going to prioritize that as an official add-on to work on. By using WP_CLI commands or other hooks/filters available, those users should have enough flexibility to craft custom workflows to achieve that.

Checking your PR again, it looks to just handle 301's, to the wp2static_list_redirects implementation should solve your problem. This will report 301s just as your WordPress site encounters them, so as long as the URL is within your site and WordPress responds to it as expected, you shouldn't need to do anything else.

I'll close this PR for now and please let me know if the recent redirects handling doesn't solve the issue in your use case.