WP-for-Church / Sermon-Manager

Sermon Manager for WordPress is the #1 plugin for churches who want to manage their sermons easily and missionally.
https://wordpress.org/plugins/sermon-manager-for-wordpress/
GNU General Public License v2.0
44 stars 34 forks source link

Sermons should save content and excerpt into proper places #184

Open nikola3244 opened 6 years ago

nikola3244 commented 6 years ago

cc-ing @robertmain

robertmain commented 6 years ago

Related to #183

robertmain commented 6 years ago

Just had a bit of a revelation with this one. I realised what post_content is being used for.

It looks like WP search doesn't search wp_postsmeta by default. The workaround for this appears to be to embed that data into post_content and then put the actual sermon text elsewhere (in this case in sermon_description). I'm wondering if something can be done to remedy this in a better way.. 🤔

nikola3244 commented 6 years ago

Yessssss, that's it! I completely forgot why I did it, until now. :laughing:

If you go back through commit history (and support forum threads) - I was saying exactly that - that we are making WP search work (it's also in changelog)

robertmain commented 6 years ago

I guess my next question is where do we go from here? I'd say we have a few different options:

  1. Do nothing. Keep things as they are. Actual post content goes into wp_postsmeta as sermon_description. I'd advise against this if at all possible not least because it would make #190 more difficult

  2. Try and intercept WP search and bolt-on support for searching wp_postsmeta. Not ideal, but I guess there are worse solutions

  3. Drop first class support for search, put our content in post_content and recommend that users install a third-party search plugin such as Relevanssi or some such. This would be my preferred solution because it would be the cleanest imho (though I appreciate you do have obligations to your users to support existing features).

I suspect the second option would be rather difficult to do well because of the fact that WP search does full table scans and has no indexing support.

robertmain commented 6 years ago

Btw, "save into post_excerpt instead of sermon_excerpt" can be marked as complete on this issue

nikola3244 commented 6 years ago

Thanks :wink:

Let me know if you are waiting on me on something, I may have missed it in the ton of emails

robertmain commented 6 years ago

I'd be interested on hear your thoughts on the previous comment. https://github.com/WP-for-Church/Sermon-Manager/issues/184#issuecomment-386159419

nikola3244 commented 6 years ago

Hahah, I knew that there was one, and it was right in front of me :laughing:

Okay:

  1. That's not a bad idea, if second one is too complicated to be implemented. I'm just not sure what issues would be with #190?
  2. I like that idea, and this filter looks promising
  3. I really don't want to require users to use 3rd party plugins to have some basic functionality of the plugin
robertmain commented 6 years ago

Well, I'd prefer not to do option 1 if possible since it isn't really a solution. Though, if we do end up doing that I guess we can close this issue.

That leaves us with the second and third options. The third is probably the cleanest solution, though like I said I understand your position of not wanting to require users to use 3rd party plugins.

The second is a workaround, but I'll look into it and see how things are.

PS: We can probably get rid of the first checkbox in the OP as it's a duplicate.

nikola3244 commented 6 years ago

Alright, if we do second one, we'll just need to add another bit to the SQL query, doesn't seem too hard.

Regarding third option, if you'd like that option because of (possible current) incompatibility with that plugin - let's just make it compatible :wink: (open an issue, if there's a bug)

PS: First and third checkboxes aren't duplicates. First one is about removing plain text rendering of the Sermon Manager's view, second one is about saving the content into the same place

robertmain commented 6 years ago

My only concern is potential performance problems... We'll see I guess...