Jump to content

Welcome, Guest!

By registering with us, you'll be able to discuss, share and private message with other members of our community.

musicmaster

Serious bug in category form

Recommended Posts

In the bleeding edge the category form shows only one group. The consequence is that when you save a category it will end up belonging to only one group: 1=default. This applies both to new groups and to groups that used to work but are saved after some change. 

For registered customers this means that they will see a screen that says that they are not allowed to access this category.

upgrade6.jpg.789def618c326577d271c7911061da9d.jpg

Edited by musicmaster

Share this post


Link to post
Share on other sites

I don't have this issue, all groups are displayed correctly. 

image.png.413fc6de6e3583e92230c7cd6baf177a.png

Thirtybees uses this sql to retrieve list of groups:

SELECT DISTINCT g.`id_group`, g.`reduction`, g.`price_display_method`, gl.`name`
FROM `tb_group` g
LEFT JOIN `tb_group_lang` `gl` ON g.`id_group` = gl.`id_group` AND gl.`id_lang` = 1 
INNER JOIN tb_group_shop group_shop ON (group_shop.id_group = g.id_group AND group_shop.id_shop = 1)
ORDER BY g.`id_group` ASC

If the group exists, then the only explanations for it not to be displayed is that it is not associated with your shop

Share this post


Link to post
Share on other sites

This shop dates from PS 1.4 and since its transition to 1.5 it has had one entry in the ps_group_shop table. If you have suddenly changed the rules that this becomes a problem than you can expect more people complaining.

But thanks for pointing out what is likely the problem.

Edited by musicmaster

Share this post


Link to post
Share on other sites

@musicmaster

Maybe that there' s been a little misunderstanding. I suppose you are confusing the database tables group_shop (assignment of customer group ids to shop id) and shop_group (IDs of shops, entry default for 1 shop)

Share this post


Link to post
Share on other sites
5 minutes ago, Occam said:

@musicmaster

Maybe that there' s been a little misunderstanding. I suppose you are confusing the database tables group_shop (assignment of customer group ids to shop id) and shop_group (IDs of shops, entry default for 1 shop)

I explicitly mentioned ps_group_shop. 

ps_shop_group has nothing to do with groups. It is about bundling shops in multishop for common stock.

The one entry I have in ps_group_shop was created by the Prestashop update process. So I wouldn't be surprised when every shop that old had the same.

Edited by musicmaster

Share this post


Link to post
Share on other sites

Yes, I read what you had mentioned. 😊 But you also wrote:

2 hours ago, musicmaster said:

This shop dates from PS 1.4 and since its transition to 1.5 it has had one entry in the ps_group_shop table.

Well, I never upgraded from 1.4 to 1.5 but started 1.5 with a fresh installation. So, afaik in Prestashop 1.5 as well as 1.6 and also in tb there are as many entries in the table ps_/tb_group_shop  as there are customer groups. But since I assume that you, as a database expert, know this for sure, you may have expressed yourself misleadingly. What exactly do you mean with the following statement: 

1 hour ago, musicmaster said:

The one entry I have in ps_group_shop was created by the Prestashop update process.

The tb update process? Or really the PrestaShop upgrade to 1.6? I'm a bit clueless.

Share this post


Link to post
Share on other sites
10 minutes ago, Occam said:

Yes, I read what you had mentioned. 😊 But you also wrote:

Well, I never upgraded from 1.4 to 1.5 but started 1.5 with a fresh installation. So, afaik in Prestashop 1.5 as well as 1.6 and also in tb there are as many entries in the table ps_/tb_group_shop  as there are customer groups. But since I assume that you, as a database expert, know this for sure, you may have expressed yourself misleadingly. What exactly do you mean with the following statement: 

The tb update process? Or really the PrestaShop upgrade to 1.6? I'm a bit clueless.

Prestashop 1.4 is monoshop. So it doesn't have _shop tables. They are added when you upgrade to 1.5. And that upgrade process created tables with only one entry - connecting shop 1 with group 1. 

Share this post


Link to post
Share on other sites
2 minutes ago, musicmaster said:

Prestashop 1.4 is monoshop. So it doesn't have _shop tables. They are added when you upgrade to 1.5. And that upgrade process created tables with only one entry - connecting shop 1 with group 1. 

I don't understand what the problem is.

Obviously, when ps14 shop was migrated 1.5, only entries for shop 1 were created in all *_shop tables. That's because, at the time of migration, only one shop existed. So this migration seems totally legit to me.

If your client created some additional shops afterwards, but failed to associate customer groups with these new shops,... then it's a human error, not a system one. 

Or did I miss anything?

Share this post


Link to post
Share on other sites

Yes

5 minutes ago, datakick said:

I don't understand what the problem is.

Obviously, when ps14 shop was migrated 1.5, only entries for shop 1 were created in all *_shop tables. That's because, at the time of migration, only one shop existed. So this migration seems totally legit to me.

If your client created some additional shops afterwards, but failed to associate customer groups with these new shops,... then it's a human error, not a system one. 

Or did I miss anything?

Sure, there was only one shop. But there were already 3 groups in 1.4. And there was only one entry created: for group 1.

Share this post


Link to post
Share on other sites

@musicmaster C'mon, I really don't need any tutoring, because I know 1.4 as well.  The multishop feature required a table like shop_group from 1.5 on. 

So you really claim that the table ps_group_shop has only one entry at all in 1.5 and 1.6? Or do you mean one entry per group? Or did you want to say that the former entries were replaced by jst one during the upgrade process?  

Share this post


Link to post
Share on other sites
11 minutes ago, Occam said:

So you really claim that the table ps_group_shop has only one entry at all in 1.5 and 1.6? Or do you mean one entry per group? Or did you want to say that the former entries were replaced by jst one during the upgrade process?  

Yes, I really claim that my ps_group_shop has only one entry at all. And I have kept all the old versions of the database and they all have it so. And it has never given a problem - until recently.

Share this post


Link to post
Share on other sites

It's very strange that these missing shop associations would not give you any errors or issues. But sure, it's possible you have used your shop in a way that such a bug wasn't encountered.

But that doesn't mean it was correct.

In fact, that itself should be considered a bug -- since these groups were not associated with your shop, they shouldn't be visible, or usable at all. Definitely non on frontend. And in many context on backend as well.

Share this post


Link to post
Share on other sites

Maybe this problem was caused by the following change in the class Group.php introduced with Bleeding Edge:


    public static $definition = [
        'table'     => 'group',
        'primary'   => 'id_group',
        'multilang' => true,
        'fields'    => [
            'reduction'            => ['type' => self::TYPE_FLOAT, 'validate' => 'isPercentage', 'size' => 17, 'decimals' => 2, 'dbDefault' => '0.00'],
            'price_display_method' => ['type' => self::TYPE_INT, 'validate' => 'isPriceDisplayMethod', 'required' => true, 'dbType' => 'tinyint(4)', 'dbDefault' => '0'],
            'show_prices'          => ['type' => self::TYPE_BOOL, 'validate' => 'isBool', 'dbDefault' => '1'],
            'date_add'             => ['type' => self::TYPE_DATE, 'validate' => 'isDate', 'dbNullable' => false],
            'date_upd'             => ['type' => self::TYPE_DATE, 'validate' => 'isDate', 'dbNullable' => false],

            /* Lang fields */
            'name'                 => ['type' => self::TYPE_STRING, 'lang' => true, 'validate' => 'isGenericName', 'required' => true, 'size' => 32],
        ],
        'keys' => [
            'group_shop' => [
                'id_shop' => ['type' => ObjectModel::KEY, 'columns' => ['id_shop']],
            ],
        ],

    ];

 

Share this post


Link to post
Share on other sites

This 'keys' definition is used for database upgrades, only. Unused in normal operations.

Share this post


Link to post
Share on other sites

This was caused by this commit: https://github.com/thirtybees/thirtybees/commit/469a569f8c98660280e3921ae277fede7d9dedc1

I would say it's not a bug, it's an expected behaviour. The group list was correctly restricted according to current shop restriction. If customer groups are supposed to be displayed in some shop context, they must be enabled for that context. In this case, they weren't. Only one group was associated with the shop. Whether this was caused by faulty migration from 1.4 to 1.5 is irrelevant. 

Share this post


Link to post
Share on other sites
8 hours ago, datakick said:

I would say it's not a bug, it's an expected behaviour. The group list was correctly restricted according to current shop restriction. If customer groups are supposed to be displayed in some shop context, they must be enabled for that context. In this case, they weren't. Only one group was associated with the shop. Whether this was caused by faulty migration from 1.4 to 1.5 is irrelevant. 

That is a choice and I can live with that.

But I want to share my impression that Prestashop after the transition to 1.5 may have concluded that they had overdone it. That too many things had shop options and that as a consequence the software became too complex and too hard to understand for people who worked with it. And that as a consequence the development of modules might suffer. 

They couldn't undo the addition of the ps_group_shop table. That might break modules. But I doubt that is a coincidence that my shop always kept working with just one entry in this table.

Share this post


Link to post
Share on other sites
7 hours ago, musicmaster said:

That is a choice and I can live with that.

But I want to share my impression that Prestashop after the transition to 1.5 may have concluded that they had overdone it. That too many things had shop options and that as a consequence the software became too complex and too hard to understand for people who worked with it. And that as a consequence the development of modules might suffer. 

They couldn't undo the addition of the ps_group_shop table. That might break modules. But I doubt that is a coincidence that my shop always kept working with just one entry in this table.

This really peak my interest, so I tried to remove some tb_group_shop entries. And indeed, all functionality I have tested worked correctly. Well, it worked. I can't say it worked correctly, since it should not have worked 🙂From this point of view, one might conclude that this table is indeed 'too much'.

On the other hand, there are users that uses multistore who actually need this functionality to work. For example, the commit above was provided by community member, because he needed the option to show only customer groups associated with particular shop.

Overall, it looks to me that there are plenty of forgotten places in the codebase where group/shop association check is missing. That should be fixed. Of course, this will create problems for you guys that migrated from 1.4 -- but the problem is there even now, so it's not a big deal. The fix is easy, and I don't think there are many uses that don't have entries in group_shop table.

I will create github issue so this not gets forgotten. I don't think it will be fixed anytime soon, though

https://github.com/thirtybees/thirtybees/issues/1102

Share this post


Link to post
Share on other sites

For Prestools I reverse engineer things and I found several times that Prestashop had implemented things more simple than I had expected from looking at the database. I can't remember concrete examples but you should expect more of this kind of issues. 

Note also that the great majority of the shops are not multishop. For them all those extra checks make the code just more complicated and slower.

Share this post


Link to post
Share on other sites
7 minutes ago, musicmaster said:

Note also that the great majority of the shops are not multishop. For them all those extra checks make the code just more complicated and slower.

But for the multi-stores, these checks make the code works correctly. What should we strive for -- the speed, or correctness? For me, this is no brainer. 

If we were discussing this in 1.4 I would probably say don't implement multistore. But we are not there. Thirtybees inherited this feature, so we have to support this.

Share this post


Link to post
Share on other sites
38 minutes ago, datakick said:

But for the multi-stores, these checks make the code works correctly. What should we strive for -- the speed, or correctness? For me, this is no brainer. 

Depends on what you want. You can make multishop as complicated as you want. The same applies to ASM. There is no limit to complexity of the needs of some customers. For some even Magento needs to be heavily customized.

My motto would be: pick your fights. Make a deliberate choice what you want to include and what not. And don't include something just because one customer asked for it.

If you don't want to support features you can always choose to leave them in the database but to make them inaccessible in the software interface.

  • Like 2

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×
×
  • Create New...