Nexus Group::hasPackages permission check

[inkredible]

Hi,

I am writing a plugin for the marketplace here which will display a different store layout/listing. Apparently I am not completely understanding how Node Models work.

 

It's basically about these lines which I have commented:

<!-- Get all packages which the member is allowed to view -->
{{$packages = \IPS\nexus\Package::roots();}}

{{foreach \IPS\nexus\Package\Group::roots() as $group}}
  	<!-- Check if this group has packages and if member is allowed to view at least one package -->
	{{if $group->hasPackages()}}
		Show package
	{{endif}}

{{endforeach}}

What I have expected to happen is described in the above comments, but what actually happened is that it apparently didn't check if the loggedIn member is allowed to view the package. It also showed the group even though all packages of this group have been set to "Don't show in store". My expectations result from the PHP Doc for the according functions. Am I doing something wrong or did I missunderstand the documentation?

[bfarber]

I cannot see from your code why packages that aren't permitted for the current viewing user would be shown

[inkredible]

2 hours ago, bfarber said:

I cannot see from your code why packages that aren't permitted for the current viewing user would be shown

Is there a chance that you could check this on my page? I don't know what I could do to further investigate the cause of that: https://streamable.com/t32pm

Maybe you can even reproduce this issue if you set your commerce products (1 group has 1 product which has the option "Show in Store" disabled) up like I did it.

 

That would be the whole nexus\front\store\index template:

{template="pageHeader" group="global" app="core" params="\IPS\Member::loggedIn()->language()->get('store')"}

{{if $credits->amount->isGreaterThanZero()}}
    <div class='ipsMessage ipsMessage_general'>
        {lang="store_credit_message" sprintf="$credits"}
    </div>
{{endif}}

<section class='ipsSpacer_top'>  
  <!-- Get all packages which the member is allowed to view -->
  {{$packages = \IPS\nexus\Package::roots();}}
  
  {{foreach \IPS\nexus\Package\Group::roots() as $group}}
      <!-- Check if this group has packages and if member is allowed to view at least one package -->
      {{if $group->hasPackages()}}
      <h2 class='ipsType_sectionHead'>{$group->_title}</h2>
      <hr class='ipsHr'>  
      <div class='ipsCarousel ipsClearfix' data-ipsCarousel data-ipsCarousel-showDots>
          <ul class='cNexusCarousel cNexusCategory_grid ipsClearfix' data-role="carouselItems">
            {{if count($packages)}}
              {{foreach $packages as $package}}
                  {{$id = $package->id;}}
                  {{if $package->group === $group->id}}
                      {template="packageBlock" group="store" params="$package, TRUE, TRUE"}
                  {{endif}}
              {{endforeach}}
            {{endif}}
        </ul>        
          <span class='ipsCarousel_shadow ipsCarousel_shadowLeft'></span>
          <span class='ipsCarousel_shadow ipsCarousel_shadowRight'></span>
          <a href='#' class='ipsCarousel_nav ipsHide' data-action='prev'><i class='fa fa-chevron-left'></i></a>
          <a href='#' class='ipsCarousel_nav ipsHide' data-action='next'><i class='fa fa-chevron-right'></i></a>
      </div>
      {{endif}}
  {{endforeach}}  
</section>

 

[Mark]

You'll need to do the permission check explicitly.

				$select = \IPS\Db::i()->select( '*', 'nexus_packages', array(
					array( 'p_group=?', $category->id ),
					array( 'p_store=1' ),
					array( "( p_member_groups='*' OR " . \IPS\Db::i()->findInSet( 'p_member_groups', \IPS\Member::loggedIn()->groups ) . ' )' )
				), $sortBy, array( ( $currentPage - 1 ) * static::$productsPerPage, static::$productsPerPage ), NULL, NULL, \IPS\Db::SELECT_SQL_CALC_FOUND_ROWS );
				$packages = new \IPS\Patterns\ActiveRecordIterator( $select, 'IPS\nexus\Package' );

 

[inkredible]

12 hours ago, Mark said:

You'll need to do the permission check explicitly.

				$select = \IPS\Db::i()->select( '*', 'nexus_packages', array(
					array( 'p_group=?', $category->id ),
					array( 'p_store=1' ),
					array( "( p_member_groups='*' OR " . \IPS\Db::i()->findInSet( 'p_member_groups', \IPS\Member::loggedIn()->groups ) . ' )' )
				), $sortBy, array( ( $currentPage - 1 ) * static::$productsPerPage, static::$productsPerPage ), NULL, NULL, \IPS\Db::SELECT_SQL_CALC_FOUND_ROWS );
				$packages = new \IPS\Patterns\ActiveRecordIterator( $select, 'IPS\nexus\Package' );

 

What is hasPackages() then good for? As far as I can see it also checks for permissions, the documentation sounds like that's exactly what I wanted and tbh I would be so happy if it would return exactly this :

/**
 * Does this group have packages?
 *
 * @param  \IPS\Member|NULL|FALSE $member    The member to perform the permission check for, or NULL for currently logged in member, or FALSE for no permission check
 * @param  mixed              $_where    Additional WHERE clause
 * @return bool
 */
public function hasPackages( $member=NULL, $_where=array() )
{
   return ( $this->childrenCount( $member === FALSE ? FALSE : 'view', $member, NULL, $_where ) > 0 );
}

 

[Mark]

It's probably leftover from a previous use. Nothing actually calls it.

[inkredible]

16 hours ago, Mark said:

It's probably leftover from a previous use. Nothing actually calls it.

That's sad, maybe you should remove or fix it otherwise. Spent many quite some time into figuring out what I was doing wrong :(.

Actually you do use it in your store/index template (this lists categories which may be empty for users without privileges to see the packages inside of that group):

<section>
    <ul class='ipsGrid ipsGrid_collapsePhone' data-ipsGrid data-ipsGrid-minItemSize='200' data-ipsGrid-maxItemSize='350' data-ipsGrid-equalHeights='row'>
        {{foreach \IPS\nexus\Package\Group::roots() as $group}}
            <li class='ipsGrid_span4 cNexusCategoryBlock'>
                <a href='{$group->url()}' data-grid-ratio='40' {{if $group->image}}style="background-image: url( '{expression="str_replace( array( '(', ')' ), array( '\(', '\)' ), $group->image )"}' );"{{endif}}>
                    <h2 class='ipsType_sectionHead ipsTruncate ipsTruncate_line'>{$group->_title}</h2>
                </a>
            </li>
        {{endforeach}}
    </ul>
</section>

Fixing these two functions would help me/other devs to create much more creative store themes. I don't like the way how IPS lists the products. Since I am writing just a plugin I can't do a codehook to fix this either. If it's planned to fix these functions please let me know, otherwise I will likely create an application instead so that I am not running into these bugs on my website.

Quick explanation why it is so important for me: We are eager to add more payment methods and obviously we test them by adding products/categories with these payment methods. Only Administrators are allowed to see them but users sent emails and asked a lot why they can't access these "test products". Other customers may have different reasons why it's important to them.

[bfarber]

10 hours ago, inkredible said:

That's sad, maybe you should remove or fix it otherwise. Spent many quite some time into figuring out what I was doing wrong :(.

Actually you do use it in your store/index template (this lists categories which may be empty for users without privileges to see the packages inside of that group):

<section>
    <ul class='ipsGrid ipsGrid_collapsePhone' data-ipsGrid data-ipsGrid-minItemSize='200' data-ipsGrid-maxItemSize='350' data-ipsGrid-equalHeights='row'>
        {{foreach \IPS\nexus\Package\Group::roots() as $group}}
            <li class='ipsGrid_span4 cNexusCategoryBlock'>
                <a href='{$group->url()}' data-grid-ratio='40' {{if $group->image}}style="background-image: url( '{expression="str_replace( array( '(', ')' ), array( '\(', '\)' ), $group->image )"}' );"{{endif}}>
                    <h2 class='ipsType_sectionHead ipsTruncate ipsTruncate_line'>{$group->_title}</h2>
                </a>
            </li>
        {{endforeach}}
    </ul>
</section>

Fixing these two functions would help me/other devs to create much more creative store themes. I don't like the way how IPS lists the products. Since I am writing just a plugin I can't do a codehook to fix this either. If it's planned to fix these functions please let me know, otherwise I will likely create an application instead so that I am not running into these bugs on my website.

Quick explanation why it is so important for me: We are eager to add more payment methods and obviously we test them by adding products/categories with these payment methods. Only Administrators are allowed to see them but users sent emails and asked a lot why they can't access these "test products". Other customers may have different reasons why it's important to them.

Mark was referring to the hasPackages method I believe, not roots() (which is a central method available to any type of node and is used extensively throughout the Suite in various fashions).

You could always write plugins to add extra methods to the store classes to perform the checks you need. If you are comfortable enough writing an entire application to handle your storefront, I'd think it would be simpler to just add a few extra methods to the Commerce classes if you feel certain things are missing.

[inkredible]

1 hour ago, bfarber said:

Mark was referring to the hasPackages method I believe, not roots() (which is a central method available to any type of node and is used extensively throughout the Suite in various fashions).

You could always write plugins to add extra methods to the store classes to perform the checks you need. If you are comfortable enough writing an entire application to handle your storefront, I'd think it would be simpler to just add a few extra methods to the Commerce classes if you feel certain things are missing.

Yes I guessed so, hence it does make sense to fix the hasPackages method and check each category to be listed against "hasPackages()" so that you don't list categories which don't have packages for the loggedIn user. Do you intend to fix it?

I just figured out that it's possible to write a code hook when creating plugins, that's great. 

[inkredible]

On 23.3.2017 at 3:08 PM, inkredible said:

Yes I guessed so, hence it does make sense to fix the hasPackages method and check each category to be listed against "hasPackages()" so that you don't list categories which don't have packages for the loggedIn user. Do you intend to fix it?

Bump. Right now you show categories which will appear "empty" for users without needed permissions for the contained packages. You could use the fixed hasPackages() function to take care of that problem.