Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When all the combinations do not have the same attributes, their prices in FO are incorrect and some combinations cannot be selected #22661

Open
AliShareei opened this issue Jan 4, 2021 · 4 comments
Labels
1.7.6.9 Affects versions Bug Type: Bug Combinations Product type: issue about products with combinations FO Category: Front Office Minor Severity: minor bug > https://build.prestashop.com/news/severity-classification Old Products Page Concerns the old product page PR available Solution: issue is being addressed Ready Status: Issue is ready to be worked on Taxes and Prices Component: Which BO section is concerned Verified The issue has been reproduced

Comments

@AliShareei
Copy link
Contributor

AliShareei commented Jan 4, 2021

Describe the bug

When product combinations are asymmetric and irregular. Usually, the combinations are not displayed correctly and even the prices may not be displayed correctly. On the other hand, when you want to add a new combination that has a new attribute, may you have many problems because u need to edit older combinations or even deleted and create a new one to make sure everything is alright on the product page. To better understand, I upload two images that show a bug that has fixed. (you can see in other cases.)

I created this PR for this problem #22639

Expected behavior

I expect PrestaShop to be more careful in displaying combinations. As far as the customer can choose.
Also have more flexibility in adding and managing combinations and do not have to change or even delete other combinations every time.

finally you can check PR #22639

Steps to Reproduce

Steps to reproduce the behavior:

  1. Go to admin
  2. Edit one product
  3. add some product attribute with a different combination
  4. go to the product page and see what happens!

Screenshots

|Fix-Attribute| Fix-Attribute2

Additional information

  • PrestaShop version: 1.7.7.0
  • PHP version: 7.1
@hibatallahAouadni
Copy link
Contributor

Hello @AliShareei

Thanks for your issue and your PR 😉
I managed to reproduce the issue with PS1.7.6.9, PS1.7.7.0 and PS1.7.7.1_build.1, see the attached screen record below:

https://drive.google.com/file/d/1-y-0Lt9dyA6pA8TR8xAWWE479ZcvnIuR/view

I’ll add this to the debug backlog so that it’s fixed.

Thanks!

@hibatallahAouadni hibatallahAouadni added 1.7.6.9 Affects versions 1.7.7.0 Affects versions Bug Type: Bug Combinations Product type: issue about products with combinations FO Category: Front Office Minor Severity: minor bug > https://build.prestashop.com/news/severity-classification PR available Solution: issue is being addressed Old Products Page Concerns the old product page Taxes and Prices Component: Which BO section is concerned Ready Status: Issue is ready to be worked on labels Jan 5, 2021
@marionf marionf changed the title Product Attributes in Product Page has bug when combinations are different. When all the combinations do not have the same attributes, their prices in FO are incorrect and some combinations cannot be selected Jan 28, 2021
@AliShareei
Copy link
Contributor Author

Hi guys,

@hibatallahAouadni
@marionf
@PierreRambaud
@jolelievre
@Progi1984

Almost more than 15 months ago I created this issue with an available PR in my opinion that PR is really good to cover this issue and problem but your team just doesn't accept it because of a minor problem.

it's like you fix a really big problem I mean 98% of that but you prefer to not apply because of a really small problem that users can manage on the admin panel like every other case.

PR: #22639

You should already know as a contributor I should keep my PR very light with small changes because if I go far and make huge changes on many files that may cause some problems in other cases which I believe makes more time to approve.

Unfortunately that PR closed and nothing happened from that time.

It is not my job to make sure that every problem I report or every PR I submit will eventually succeed to the core or problem will be fixed. I assume if you are an official member of the Prestashop team it would be your job, and I have tried to make it easier.

Does this way of following issues mean that I no longer have to spend time with PrestaShop?
If you think my PR was not good enough, why nothing has been done?
I really want to help, but apparently, you are not interested in pursuing and you despise the efforts of others.

@matks
Copy link
Contributor

matks commented Mar 22, 2022

Dear @AliShareei

First of all I would like to acknowledge what happened on this topic.

You did a great work by opening this issue and submitting PR #22639. This is a lot of work.

The PR was reviewed by @Progi1984 @NeOMakinG @atomiix. This is also work.
The PR was tested by QA team @khouloudbelguith and an issue was found. This is also work. Some questions were asked to @marionf and she answered, that's also work.

So as you can see a lot of people invested time in this topic, and the fact the contribution was not able to go to the end is a loss not only for you but for everybody who worked on it 😞 .

... is it a loss? Actually the work you did is still great and useful. If someone reads this github issue, he can look at your PR like a potential solution for her/him. So your PR is going to maybe help one thousand people fix their shop, that is great! and this is possible because you shared your work on this repository. This is something that can only happen because it's open source ❤️

Back to the point, you're disappointed because you submitted a PR, it went through review and QA and later QA team found an unwanted behavior and we disagree on whether the contribution can be merged although it does not solve this issue. As you mentioned, you think the PR solved at least 98% of the problem and you're disappointed because of the missing 2% we do not merge it.

Yes, these 2% are important to us. We have a high expectation for contributions, and we acknowledge that. You submitted a PR but the topic behind was simply bigger than expected. The topic was too big for this PR, at this moment, and you do not wish to invest more time to handle the remaining 2% . It's all right, we know PrestaShop is complex. PrestaShop is complex and we have high expectations about the contributions because we think this is how the software can grow and become better. Quality comes with a high price. We do not want to merge PRs we do not think are complete enough because we did it in the past and it brought issues such as regressions or trade-offs not well used. So now we are very strict on the level of completion of merged PRs. It's more expensive, it's harder to do these PRs, but the software benefits greatly from that. As I said, the quality of the software comes from raising the level of expectation on any code change which means Pull Requests.

So we acknowledge your work, we thank you for pushing your PRs (and as I said it might be very helpful to some people) but unfortunately if you cannot meet the high level of expectation we put on PR we cannot accept your contribution. Yes, contributing to PrestaShop can be hard sometimes because of that 🏙️ . But PrestaShop now is so complex and so big after all these years that this level of expectation is required to keep the software grow and maintain its level of quality.

And we go through the same level of expectations. The maintainer team has to do a huge work to submit Pull Requests that get merged. It's hard for us too 😄 .

There's an article I think is quite well written about this topic: https://github.community/t/creators-contributors-and-collaborators/10219 It mentions the topic of a maintainer does not accept a contribution.

It is not my job to make sure that every problem I report or every PR I submit will eventually succeed to the core or problem will be fixed. I assume if you are an official member of the Prestashop team it would be your job, and I have tried to make it easier.

Actually it is exactly your job, yes. And this is explained in the article above:

"The way open source works is that everyone contributes based on their own needs. If your needs don’t align with anyone else’s, then it may be completely up to you to figure out how to fix or implement something. Even if you need the same thing as other people involved in the project, that doesn’t mean that what you want will be others’ top priority."

If it is important for you that the above bug is fixed, then you must invest the time to do it and meet the expectations of the maintainer team. Because it's important to you.

And we do the same for topics that are important to us. We choose them and invest the time to fix them to the fullest.

If a topic is important for you, you must push it because nobody else will (unless it's important for them too). This is how open source works.

The fact we are part of the PrestaShop team does not impact our contributions. When I submit a PR, it's processed like any other PR I am a regular contributor just like you 👨‍🚀

Does this way of following issues mean that I no longer have to spend time with PrestaShop?

If you cannot submit PRs that meet our level of expectations ; then yes you do not have to submit new PRs. It's all right we acknowledge it's hard. But then the topics important to you will not go forward. Open source is great because it gives you the opportunity to actually change things in the software you use (you cannot do this with a SaaS product) but it's not easy.

If you think my PR was not good enough, why nothing has been done?

Because doing something on this PR is work. And on our side we already have plenty of other work to do, things that we identified as important for us. It's mostly the 8.0.0 roadmap . We cannot pause our work to finish yours. If one day this topic becomes important to us, we'll take it. We probably will try to include your commits in a new Pull Request in order to acknowledge your work. But today this topic is not our priority, we have other topics to work on so we'll do nothing on your PR.

I really want to help, but apparently, you are not interested in pursuing and you despise the efforts of others.

We do not despise your work as I said, and please also note that the work of QA, PM and developers on your PR is work too. So we actually worked on your PR although it's not something we wanted to push because we have the duty to validate incoming contributions. But we do not have the duty to push them.

We will welcome your contributions but we will require you push the same level of quality that we push ourselves. We will ask you do as much work as we do, not more, not less. We consider you as an equal and not someone we need to help. You want to help? That's great, you can push PRs and go through the same level of expectations that we do. But you will have to push "as hard" and "as high" as we do, because this is how we can, all of us, together, push the software to higher levels.

As said by javiereguiluz maintainer of EasyAdmin, "I don’t work for you [...]. I can work with you to solve the issues that you are experiencing, but you need to do most of the work"

Here are two more items that you could find interesting to read, I wrote them:

And in order to finish this long message, again you did a great work by opening this issue and submitting PR #22639. This is a lot of work and bravo.
The PR was reviewed by @Progi1984 @NeOMakinG @atomiix. This is also work and bravo to them.
The PR was tested by QA team @khouloudbelguith and an issue was found. This is also work. Some questions were asked to @marionf and she answered, that's also work. Bravo to them.

(little note: I spent almost 40 minutes writing the above message, this is work too 😄 but I think you deserve an answer)

@AliShareei
Copy link
Contributor Author

Hi @matks Thanks for your good response, I appreciate that and this is very valuable to me.
Also, thank you for following up on another issue I created earlier. #26606

About the subject, you really missed the point of what I said. My disappointment is not about my PR, is about unresolved problems that go on for months and years, even though they have a solution, they remain intact.

If my PR is approved, which is great, but what is more important is the final result. Was the problem solved or not?

What I see is a problem that was reported more than a year ago, and a solution has been proposed that could easily be helpful. I mean really useful.

But what I finally see is how many people have entered the subject, commented, and left. We need an output. What is the value of a comment without a result? Has anyone tried to make a better PR?

In terms of quality, what is quality if it means not doing something? There is no quality when nothing is done.

When the current situation is a disaster and the solution is as good (for example) as 98%, but we prefer to do nothing for just 2% (which user can manage by sorting attributes on the admin panel), it does not mean quality. This means that we are afraid of choice and prefer to leave it even for nearly 1.5 years.

If it is important for you that the above bug is fixed, then you must invest the time to do it and meet the expectations of the maintainer team. Because it's important to you.

Basically, I wrote this PR lightly, In fact, I just made the previous code in the core better and cleaner, Which actually created a useful output. I suspect no one of the original developers carefully reviewed the code because if you did, you would find that the structure has not changed fundamentally that we don't need to be afraid of change.

However, I believe that solving a comprehensive problem requires deeper insight and vision into the software, and that can only be done by you and others as the main developers.

even may need to modify countless files to solve the problem. Do you think an out-of-batch developer can do dozens of files on their own without the supervision and deeper vision of the development team?

I'm not sure if it will end well if I do it patiently and deeply. I have experienced it before, it is a big risk that a lot of time is wasted because it may be out of the main view and the main path.

If you cannot submit PRs that meet our level of expectations; then yes you do not have to submit new PRs. It's all right we acknowledge it's hard. But then the topics important to you will not go forward. Open source is great because it gives you the opportunity to actually change things in the software you use (you cannot do this with a SaaS product) but it's not easy.

Because doing something on this PR is work. And on our side we already have plenty of other work to do, things that we identified as important for us. It's mostly the #26427 . We cannot pause our work to finish yours. If one day this topic becomes important to us, we'll take it. We probably will try to include your commits in a new Pull Request in order to acknowledge your work. But today this topic is not our priority, we have other topics to work on so we'll do nothing on your PR.

I did not mean the PRs. I meant as a store manager or a module developer. If you want to choose your path based on your own priorities, regardless of the needs of the store owner or developer, it means that my presence as a store manager or developer is a low priority for you.

Let's be honest, my product is not my PR. My product is not the core of PrestaShop. This is your product. I can only do something based on your product. Maybe a contribution, maybe a module, a theme, or a business and store. We are not equal in this matter. But somewhere we have to be on one hand and in the same direction so that we can be useful to each other. When your product lags behind the reported problem for months and years, can I, as a developer, stay in this environment or have to choose my own path?

I do not put any pressure on you, I even tried to build a smooth solution. If there was a problem, I tried to solve it myself, That's why I try to make a PR for every issue I report, whether you accept it or not. but I expected something to be done and an exit to be seen.

Anyway, thank you for your time and efforts. I hope this problem will be resolved like the others and will not be more annoying.

With respect and best wishes to you

@hibatallahAouadni hibatallahAouadni added Verified The issue has been reproduced and removed 1.7.7.0 Affects versions labels Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.7.6.9 Affects versions Bug Type: Bug Combinations Product type: issue about products with combinations FO Category: Front Office Minor Severity: minor bug > https://build.prestashop.com/news/severity-classification Old Products Page Concerns the old product page PR available Solution: issue is being addressed Ready Status: Issue is ready to be worked on Taxes and Prices Component: Which BO section is concerned Verified The issue has been reproduced
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants