AdminOrdersBpost.php enhancements

[tchauviere]

• https://github.com/PrestaShop/bpostshm/blob/master/AdminOrdersBpost.php#L15-19: This verification is not required, since this file should never be called when running on 1.5+
• https://github.com/PrestaShop/bpostshm/blob/master/AdminOrdersBpost.php#L1144-1152: PHP doc should before method declaration, not inside
• https://github.com/PrestaShop/bpostshm/blob/master/AdminOrdersBpost.php#L1142-1276 Method displayListContent(): As a general comment, you will need to refactorize this part. You have too much html in this method, please use templates and smarty to make it more readable and clear. In another hand, please use “backward_compatiblity” to access globals you set at the begining of this method via $this->context;

[Stigmi]

• https://github.com/PrestaShop/bpostshm/blob/master/AdminOrdersBpost.php#L15-19: This verification is not required, since this file should never be called when running on 1.5+

  NOK, without those 2 lines at the beginning we have no 1.5+ AdminOrdersBpost at all

• https://github.com/PrestaShop/bpostshm/blob/master/AdminOrdersBpost.php#L1144-1152: PHP doc should before method declaration, not inside

  You mean the comment?

• https://github.com/PrestaShop/bpostshm/blob/master/AdminOrdersBpost.php#L1142-1276 Method displayListContent(): As a general comment, you will need to refactorize this part. You have too much html in this method, please use templates and smarty to make it more readable and clear.

  We would very much appreciate not having to redo something that is working well for 1.4+ versions. In order to achieve this result for all versions, we had to invest a lot of time and mimic the 1.4 core admin pages. We started workig with templates and smarty and it was OK for 1.5+ but 1.4 version was a no-go and we had to recreate this code in order to have the same display on all versions (which is one of our requirement). We don't find it easy to read but it's working well (+it validates!).

• In another hand, please use “backward_compatiblity” to access globals you set at the begining of this method via $this->context;

  OK this was done.

[tchauviere]

Updated comments

• https://github.com/PrestaShop/bpostshm/blob/master/AdminOrdersBpost.php#L15-19: This verification is not required, since this file should never be called when running on 1.5+

  NOK, without those 2 lines at the beginning we have no 1.5+ AdminOrdersBpost at all

  • Ok

• https://github.com/PrestaShop/bpostshm/blob/master/AdminOrdersBpost.php#L1144-1152: PHP doc should before method declaration, not inside

  You mean the comment?

  • Yes, by convention they should be just before method declaration

• https://github.com/PrestaShop/bpostshm/blob/master/AdminOrdersBpost.php#L1142-1276 Method displayListContent(): As a general comment, you will need to refactorize this part. You have too much html in this method, please use templates and smarty to make it more readable and clear.

  We would very much appreciate not having to redo something that is working well for 1.4+ versions. In order to achieve this result for all versions, we had to invest a lot of time and mimic the 1.4 core admin pages. We started workig with templates and smarty and it was OK for 1.5+ but 1.4 version was a no-go and we had to recreate this code in order to have the same display on all versions (which is one of our requirement). We don't find it easy to read but it's working well (+it validates!).

  • I understand your position, but this is clearly a no go for us, the fatest way to do it - is to keep all your variables, assign them to the template and copy past your code logic directly into the new template and making the remaining changes required (mostly find/replace)

[Stigmi]

OK, then we should escalate this ticket to bpost. It would be technically impossible for us to have the same behavior on all versions + it will take weeks to redo/retest this part.

Again, we started by doing it the "right way" before having to change to the current code because 1.4 version was too far behind.

I attach the 1.4 version of the BO, so that you can have a look at what we are talking about (tabs, dropdowns, search and filters).

[tchauviere]

Updated comments

• ~~https://github.com/PrestaShop/bpostshm/blob/master/AdminOrdersBpost.php#L1144-1152: PHP doc should before method declaration, not inside~~

  You mean the comment?

  • Yes, by convention they should be just before method declaration
  • It was not a PHP Doc element, everything is ok here, my bad.

• https://github.com/PrestaShop/bpostshm/blob/master/AdminOrdersBpost.php#L1142-1276 Method displayListContent():

  As a general comment, you will need to refactorize this part. You have too much html in this method, please use templates and smarty to make it more readable and clear.

  • For this matter, @gaillafr will contact you by email as we agreed by phone

---

I let this issue open, for the templating matter but for the all others issues we are OK

Regards,