The best of the forks #123

christianwach · 2014-10-06T16:02:07Z

#121 prompted me to create this overdue PR. See explanation there.

bp_groups_setup_nav action is part of the default theme I suspect as it's not present on my install of 1.6.4 and we don't use the default theme. changing it to bp_init did the trick, so I'm offering it up as a change. Without it, it would just 404. Thanks!

This lets me focus on just the Course-related changes without having to be distracted by unresolved issues in other parts of the plugin

…uddyPress Setting the filter to false will allow the plugin to be activated without complaining that BuddyPress isn't activated, and can be used for other similiar checks in the future

This will run at the exact same time as bp_init, but will fire when BuddyPress is installed and when it isn't

These simulate the existance of BuddyPress for places in the BPSP code that expects certain BP objects/functions to exist

…ess not activated

…t directly The call to bp_get_group_permalink() was moved because it's only used inside the if ( !empty( course[0] ) ) check, and calling it earlier leads to an undefined function error when BuddyPress is not activated

Otherwise it causes "headers already sent" warnings

…uired

Otherwise the post types couldn't be accessed directly until some other action flushed the rules

Either BPSP will call a function that's undefined (because the mock isn't setup), or BuddyPress will try to redeclare a mocked function.

Otherwise it can't be accessed and a PHP warning is thrown.

If it isn't initialized, a PHP notice will be thrown

edit_course_screen() performs a capability check and dies if it fails, so there's no need to also check capabilities in standalone_screen_handler before edit_course_screen() is called.

They can both use the same ones, so there's no need to maintain separate/overlapping lists

The default values are needed for the single template too.

Without this, a PHP warning will be thrown when bp_get_group_permalink() is called

stas · 2014-10-06T17:04:27Z

OMG, please give me some time to take a look before we merge this 🙏

christianwach · 2014-10-06T17:07:28Z

@stas Sorry, I should have copied what I said on Twitter in here:

https://twitter.com/interactivist/status/519160895248023552

Please consider this PR a bookmark for my branch!

stas · 2014-10-12T16:59:12Z

courses/courses.class.php

+		}
+
+		return $content;
+	}


This looks good, although I wonder why not register a group custom post type and use it instead of the one provided by BuddyPress? I'm not sure what are the implications for using these magic values/IDs, especially if somebody decides to migrate later to a BuddyPress installation.

/cc @christianwach

@stas This code was from a commit by @iandunn who would probably be better placed to answer your query. I did not thoroughly test the standalone version. My own view is that it would be highly unlikely for anyone to use BP Courseware without BP, let alone then convert to using BP. I agree with your suggestion - a group CPT would help conversion to a BP context should that occur.

why not register a group custom post type and use it instead of the one provided by BuddyPress

It's been a long time since I worked on that, but I'm guessing I chose that route because I didn't see any reason to implement any BuddyPress-specific features, since the point was the make it available to users who weren't running BP. You make a good point about the potential for someone migrating to BP later on, although I'm not sure if that'd be common enough to warrant the extra time.

Also, just as FYI, my entire branch was still alpha, and will probably needs some work before merging into stable BPCW.

Thanks @iandunn
I would rather merge this an improve later, rather than wait until it is stable.

stas · 2014-10-12T17:23:23Z

I left a couple of suggestions. Overall I think this is good to merge!
Since the biggest changes are relevant to the standalone feature, I think we can continue with the current approach to handle the ACL and groups support and consider changing that in nearest future with a more pragmatic alternative.

Huge thanks to @iandunn @christianwach @sboisvert and @16nsk

@lloc, @Mamaduka, if you have suggestions, please leave a comment, otherwise I will consider this as good to merge.

lloc · 2014-10-13T08:27:43Z

It would be fine for me.

christianwach · 2014-10-13T08:49:53Z

@stas Thanks for reviewing the PR. Look forward to following development!

The best of the forks 💥

Mamaduka · 2014-10-14T11:02:04Z

@stas thanks for merging this PR.

sboisvert and others added 30 commits February 4, 2013 17:55

Temporarily disabling non-course modules

c10aa53

This lets me focus on just the Course-related changes without having to be distracted by unresolved issues in other parts of the plugin

Adding bpsp_require_buddypress filter to allow running BPSP without B…

ba34a95

…uddyPress Setting the filter to false will allow the plugin to be activated without complaining that BuddyPress isn't activated, and can be used for other similiar checks in the future

Hooking bpsp_init() to init instead of bp_init

4c68aa5

This will run at the exact same time as bp_init, but will fire when BuddyPress is installed and when it isn't

Adding mock BuddyPress objects and functions

765a87d

These simulate the existance of BuddyPress for places in the BPSP code that expects certain BP objects/functions to exist

Adding standalone_screen_handler to control Courses view when BuddyPr…

8d6b752

…ess not activated

Allow courses post type to be access directly from the front end

588a6ae

Adding bp->groups->current_group to mock bp properties

9cc19a1

Short circuiting profile field registration if BuddyPress not activated

37d69c6

Setting empty groups array when BuddyPress is not activated

e5a84f0

Adding mock bp_core_admin_hook()

7f01c4a

Updating file description

b4a1055

Removing trailing whitespace after closing PHP tag

63d1706

Otherwise it causes "headers already sent" warnings

Merge remote-tracking branch 'origin/bp_not_required' into bp_not_req…

7b0962e

…uired

Checking if _GET['action'] exists before trying to access it

73b982e

Adding bp_core_get_userlink stub

ff9b441

De-registering and re-registering hook callback to avoid infinite loop

dd310d5

Checking if variables exist before referencing them

e0f3b84

Flushing rewrite rules on plugin activation and deactivation

fbad8fa

Otherwise the post types couldn't be accessed directly until some other action flushed the rules

Avoid loading the plugin when BuddyPress is being activated

17e9464

Either BPSP will call a function that's undefined (because the mock isn't setup), or BuddyPress will try to redeclare a mocked function.

Correct deactivation hook callback

f85f839

Bring global pagenow variable into function scope

8e1b481

Otherwise it can't be accessed and a PHP warning is thrown.

Initialize BuddyPress current_group object with mock value

67c0904

If it isn't initialized, a PHP notice will be thrown

Removing note about checking capabilities in standalone_screen_handler()

78bcf9d

edit_course_screen() performs a capability check and dies if it fails, so there's no need to also check capabilities in standalone_screen_handler before edit_course_screen() is called.

Combining vars for single_course and edit_course

4d1ea2a

They can both use the same ones, so there's no need to maintain separate/overlapping lists

Correcting course_permalink value

34ec38f

Adding mock stub for bp_get_group_permalink()

b4c881b

Moving BuddyPress mock values outside edit block

e8c0201

The default values are needed for the single template too.

Add mock value for BuddyPress current_group slug

87c1822

Without this, a PHP warning will be thrown when bp_get_group_permalink() is called

christianwach added 15 commits June 5, 2014 11:34

fix typo

f8a331c

add loader file

95b41c3

merge BP and non-BP loading methods

255f47a

fix unnecessary pass-by-reference calls

9302c7e

fix activation errors

b766d51

rejig loading and enable admin page when network-activated

332747e

fix misnamed filter method

02f8c0b

fix misc errors under BP1.8+

19653a3

fix warnings and errors in bibliography

356039d

revert debugging code

5ba96fd

add HTML comments to identify templates

ff73a2c

prevent page jumping when utility links are clicked

c9231c7

add HTML comment identifiers to admin templates

11089bf

remove closing PHP tags from the end of files

d8463bf

add preventDefault behaviour when slideToggling

bf77a80

christianwach mentioned this pull request Oct 6, 2014

PHP Strict Standards, Notice and Warnings #121

Open

stas reviewed Oct 12, 2014
View reviewed changes

This was referenced Oct 12, 2014

https://github.com/Courseware/buddypress-courseware/issues/121 #122

Closed

Update groups/groups.class.php #101

Closed

stas added a commit that referenced this pull request Oct 14, 2014

Merge pull request #123 from christianwach/cmw-merged

c7c26c7

The best of the forks 💥

stas merged commit c7c26c7 into Courseware:master Oct 14, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

The best of the forks #123

The best of the forks #123

christianwach commented Oct 6, 2014

stas commented Oct 6, 2014

christianwach commented Oct 6, 2014

stas Oct 12, 2014

christianwach Oct 13, 2014

iandunn Oct 13, 2014

stas Oct 14, 2014

stas commented Oct 12, 2014

lloc commented Oct 13, 2014

christianwach commented Oct 13, 2014

Mamaduka commented Oct 14, 2014

The best of the forks #123

The best of the forks #123

Conversation

christianwach commented Oct 6, 2014

stas commented Oct 6, 2014

christianwach commented Oct 6, 2014

stas Oct 12, 2014

Choose a reason for hiding this comment

christianwach Oct 13, 2014

Choose a reason for hiding this comment

iandunn Oct 13, 2014

Choose a reason for hiding this comment

stas Oct 14, 2014

Choose a reason for hiding this comment

stas commented Oct 12, 2014

lloc commented Oct 13, 2014

christianwach commented Oct 13, 2014

Mamaduka commented Oct 14, 2014