'Add to Portfolio' missing from portfolio settings

cuny-academic-commons / commons-in-a-box

Commons In A Box - A suite of community and collaboration tools for WordPress, designed especially for academic communities

http://commonsinabox.org

72 stars 14 forks source link

'Add to Portfolio' missing from portfolio settings #308

Closed bree-z closed 3 years ago

bree-z commented 3 years ago

Hi Boone,

I don’t see the 'Add to Portfolio' settings available on openlabdev.comonsinabox.org. I don’t remember if I ever actually tested this. Here's the only related ticket I could find, but it doesn't look like I tested it: https://github.com/cuny-academic-commons/commons-in-a-box/issues/200

I don't see the settings here, or on any of the other settings sub-pages: https://openlabdev.commonsinabox.org/groups/cosmo-ortizs-portfolio/admin/group-settings

Compare with openlabdev.org, where it appears here: http://openlabdev.org/groups/tsfirstname-tslastnames-eportfolio-8/admin/group-settings/ (as in the screenshot below).

Thanks!

boonebgorges commented 3 years ago

@Mamaduka Could you have a look? I wonder whether the method we use to add the settings panel on the OpenLab doesn't properly translate to the openlab-theme templates.

If this is an easy fix (my guess is yes) then we can include it in 1.2.3.

Mamaduka commented 3 years ago

Hello everyone,

Sorry, this is my mistake.

When we decided to extract the plugin, I forgot to add the associated settings panel to the "openlab-theme".

This is a small change and can be included in 1.2.3.

Mamaduka commented 3 years ago

A few questions:

Do we want the settings panel code to be a part of the theme?
Do we need an editable label for this setting?

boonebgorges commented 3 years ago

Thanks for the quick response, @Mamaduka .

Do we want the settings panel code to be a part of the theme?

Whatever we do with similar settings is fine. I can't remember :+1:

Do we need an editable label for this setting?

Oh yeah, I guess we do, since 'Portfolio' is editable. Can you take care of this too? Note that we'll need an upgrade routine.

Mamaduka commented 3 years ago

Decided to include settings panel code into the OpenLab Portfolio plugin, the feature is only available when the plugin is active.

Added help text label and included that in your upgrade routine for 1.2.3 ( 7c7f9ca ). I used the same text that we have on OpenLab, except for the help URL:

The Add to Portfolio feature saves selected posts, pages, and comments that you have authored on Course, Project, and Club sites directly to your Portfolio site.

Please let me know if this needs an update.

P.S. @boonebgorges Sorry that I had to make changes across the repos.

boonebgorges commented 3 years ago

Thanks for this, @Mamaduka.

Your changes remind me that the 'Add to Portfolio' feature has some language that references 'Portfolio' in a hardcoded way. Currently, if you change the 'Portfolio' group type label, the button still says 'Add to Portfolio' (as do the group settings panels; see https://github.com/openlab-at-city-tech/openlab-portfolio/commit/12f09363a2e182ba906823161498e0cd34435796#diff-64a00a62e8273f50ae6de852d7c9b35202c1407c1c4d949b4bfd5d01e113789dR9). I'm going to open a separate ticket to track this issue.

boonebgorges commented 3 years ago

@Mamaduka It looks like you didn't bump the version number in openlab-portfolio before creating the tag https://github.com/openlab-at-city-tech/openlab-portfolio/blob/9835b314093b2236fe053b5446f7a7bcb5c28cfe/openlab-portfolio.php#L8 This means that the CBOX updater gets stuck in a loop: it thinks it needs an upgrade, so it performs it, but then still says it needs an upgrade. Can you please bump this? Probably easiest to go straight to 1.0.2 so that you don't have to mess with the existing tag.

Mamaduka commented 3 years ago

Done.

boonebgorges commented 3 years ago

Thank you :)

bree-z commented 3 years ago

Thanks, George and Boone! This looks good except for one issue. The citation generated for the post being added to the portfolio includes html tags, e.g.:

This <a href="https://openlabdev.commonsinabox.org/language-culture-and-society/2021/07/18/test-students-post/">entry</a> was originaly posted in "Language, Culture, and Society" on July 18, 2021

See here, for example: https://openlabdev.commonsinabox.org/test-student-portfolio-4/2021/07/18/test-students-post/

Thanks!

boonebgorges commented 3 years ago

@Mamaduka Could you please have a look? I thought this might be an easy fix, but it looks like the 'portfolio_citation' metadata is escaped in the database.

From the looks of it, you're grabbing the .html() of the #citation element when submitting the Share form: https://github.com/openlab-at-city-tech/openlab-portfolio/blob/7e2c074f3eee9554baac2a3b665bb21be6a906b6/assets/js/share.js#L80 But this content is escaped when run through wp.template() to build the dialog interface.

I think the fix is to change the esc_html_e() call here https://github.com/openlab-at-city-tech/openlab-portfolio/blob/7e2c074f3eee9554baac2a3b665bb21be6a906b6/views/share/dialog.php#L41 to echo wp_kses_post(), which will ensure that citations created in the future are stored raw. It won't fix existing citations but I think that's OK since this is a new feature. Can you please think about it and let me know whether you agree with this fix?

boonebgorges commented 3 years ago

I implemented my suggested fix and it's testing properly for me.

@bree-z Please have a look on openlabdev.commonsinabox.org. Note that my fix will not affect existing items that have already been added to your portfolio, so you'll have to add a new item to see the fix in action.

bree-z commented 3 years ago

This looks good, thanks!