Sitemap filename can't exceed 32 characters #13937 #20044

irajneeshgupta · 2019-01-02T14:53:32Z

Description (*)

The Sitemap filename length validation was not added , hence added \Magento\Framework\Validator\StringLength max 32 length validation for sitemap filename and it will not trim filename exceeding 32 chars. Client side validation in filename field of form is also added to fix this issue 'validate-length maximum-length-32'. Updated saveTest unit test case.

Fixed Issues (if relevant)

magento/magento2#Sitemap filename can't exceed 32 characters #13937: Issue title : Sitemap filename can't exceed 32 characters

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds on Travis CI are green)

magento-engcom-team · 2019-01-02T14:53:38Z

Hi @irajneeshgupta. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento-engcom-team give me test instance - deploy test instance based on PR changes
@magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

miguelbalparda · 2019-01-02T18:03:01Z

Using the objectmanager doesn't seem to be the best path here, care to recheck?

irajneeshgupta · 2019-01-02T18:18:58Z

Using the objectmanager doesn't seem to be the best path here, care to recheck?

@miguelbalparda
yeah but Magento itself used objectmanager the same in above lines for path validation , hence i added it in the same way.
I will recheck it and amend my commit.
Thanks for your suggestion.

irajneeshgupta · 2019-01-03T14:06:43Z

Using the objectmanager doesn't seem to be the best path here, care to recheck?

@miguelbalparda
I rechecked and removed objectManager from last commit.
Thank you.

…to#13937.

irajneeshgupta · 2019-01-10T08:07:59Z

@miguelbalparda can you please review changes.
Thanks.

magento-engcom-team · 2019-01-13T09:14:46Z

Hi @aleron75, thank you for the review.
ENGCOM-3844 has been created to process this Pull Request

sivaschenko

Thanks for your contribution @irajneeshgupta , please see my code review notes.

sivaschenko · 2019-01-21T11:28:27Z

app/code/Magento/Sitemap/Controller/Adminhtml/Sitemap/Save.php

+    /**
+     * @var $_stringValidator
+     */
+    public $_stringValidator;


Please change properties access level to private and remove the underscore prefix from property names

Okay , I will made these changes.

sivaschenko · 2019-01-21T11:31:31Z

app/code/Magento/Sitemap/Controller/Adminhtml/Sitemap/Save.php

+     * @param \Magento\Sitemap\Model\SitemapFactory $sitemapFactory
+     */
+    public function __construct(
+        \Magento\Backend\App\Action\Context $context,


Please add dependencies in a backward compatible way, see "Adding a constructor parameter" section of Backward compatible development guide for details

okay I will update it.

sivaschenko · 2019-01-21T11:34:34Z

app/code/Magento/Sitemap/Controller/Adminhtml/Sitemap/Save.php

+    const MAX_FILENAME_LENGTH = 32;
+
+    /**
+     * @var $_stringValidator


Please correct propeties' types in phpdocs

okay I will make these changes

sivaschenko · 2019-01-21T11:35:25Z

app/code/Magento/Sitemap/Test/Unit/Controller/Adminhtml/Sitemap/SaveTest.php

+    /**
+     * @var \Magento\Framework\Validator\StringLength|\PHPUnit_Framework_MockObject_MockObject
+     */
+    protected $lengthValidator;


it's preferable to declare properties as private unless they are used by child classes

…kward compatible constructor params

irajneeshgupta · 2019-01-22T06:00:55Z

@sivaschenko

I have made changes that you recommended.
Please review.

Thanks.

magento-engcom-team · 2019-01-23T14:09:39Z

Hi @aleron75, thank you for the review.
ENGCOM-3844 has been created to process this Pull Request

irajneeshgupta · 2019-02-01T17:05:13Z

@sivaschenko
any updates on this PR.

sivaschenko · 2019-02-08T14:59:09Z

Hi @irajneeshgupta thanks for your fixes, the pull request is being processed internally.

…ta-patch-13937

ghost · 2019-03-01T08:01:51Z

Hi @irajneeshgupta, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

magento-engcom-team added Component: Sitemap Release Line: 2.3 labels Jan 2, 2019

miguelbalparda self-assigned this Jan 2, 2019

irajneeshgupta force-pushed the irajneeshgupta-patch-13937 branch from 911538e to f8654d1 Compare January 3, 2019 13:59

irajneeshgupta force-pushed the irajneeshgupta-patch-13937 branch from 21f4361 to ab2e514 Compare January 3, 2019 14:31

irajneeshgupta requested a review from sidolov January 3, 2019 14:41

irajneeshgupta force-pushed the irajneeshgupta-patch-13937 branch from f6bc76b to 71a0047 Compare January 4, 2019 11:56

String validation for filename added and updated unit test case magen…

55cf2dc

…to#13937.

irajneeshgupta force-pushed the irajneeshgupta-patch-13937 branch from 9f3bb74 to 55cf2dc Compare January 4, 2019 13:48

milindsingh requested a review from miguelbalparda January 5, 2019 06:36

milindsingh added community-insider-contribution Community Insider: Cedcommerce labels Jan 5, 2019

irajneeshgupta requested a review from nuzil January 9, 2019 06:50

irajneeshgupta requested a review from aleron75 January 11, 2019 08:46

aleron75 approved these changes Jan 13, 2019

View reviewed changes

magento-engcom-team assigned aleron75 Jan 13, 2019

magento-engcom-team added this to the Release: 2.3.1 milestone Jan 13, 2019

magento-engcom-team added the Progress: accept label Jan 17, 2019

sivaschenko self-assigned this Jan 21, 2019

sivaschenko requested changes Jan 21, 2019

View reviewed changes

sivaschenko added the Progress: needs update label Jan 21, 2019

Updated phpdocs, property access levels,removed underscore, added bac…

035e8d7

…kward compatible constructor params

irajneeshgupta force-pushed the irajneeshgupta-patch-13937 branch from c4d191d to 035e8d7 Compare January 22, 2019 05:56

aleron75 approved these changes Jan 23, 2019

View reviewed changes

irajneeshgupta removed the Progress: needs update label Jan 26, 2019

okorshenko removed this from the Release: 2.3.1 milestone Jan 28, 2019

nmalevanec added 3 commits February 25, 2019 15:02

Merge remote-tracking branch 'mainline/2.3-develop' into irajneeshgup…

417ffa2

…ta-patch-13937

Fix static tests.

e9f4981

Fix static test.

94c176f

magento-engcom-team mentioned this pull request Mar 1, 2019

Sitemap filename can't exceed 32 characters #13937

Closed

magento-engcom-team merged commit 94c176f into magento:2.3-develop Mar 1, 2019

magento-engcom-team added the Award: bug fix label Mar 1, 2019

magento-engcom-team added this to the Release: 2.3.2 milestone Mar 1, 2019

magento-engcom-team pushed a commit that referenced this pull request Mar 1, 2019

ENGCOM-3844: Sitemap filename can't exceed 32 characters #13937 #20044

169b59c

amol2jcommerce mentioned this pull request Mar 1, 2019

[Backport] Sitemap filename can't exceed 32 characters #13937 #21530

Closed

4 tasks

irajneeshgupta mentioned this pull request Mar 2, 2019

[Backport] Sitemap filename can't exceed 32 characters #13937 #21553

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sitemap filename can't exceed 32 characters #13937 #20044

Sitemap filename can't exceed 32 characters #13937 #20044

irajneeshgupta commented Jan 2, 2019 •

edited

Loading

magento-engcom-team commented Jan 2, 2019

miguelbalparda commented Jan 2, 2019

irajneeshgupta commented Jan 2, 2019 •

edited

Loading

irajneeshgupta commented Jan 3, 2019 •

edited

Loading

irajneeshgupta commented Jan 10, 2019

magento-engcom-team commented Jan 13, 2019

sivaschenko left a comment

sivaschenko Jan 21, 2019

irajneeshgupta Jan 21, 2019

sivaschenko Jan 21, 2019

irajneeshgupta Jan 21, 2019

sivaschenko Jan 21, 2019

irajneeshgupta Jan 21, 2019 •

edited

Loading

sivaschenko Jan 21, 2019

irajneeshgupta Jan 21, 2019

irajneeshgupta commented Jan 22, 2019

magento-engcom-team commented Jan 23, 2019

irajneeshgupta commented Feb 1, 2019

sivaschenko commented Feb 8, 2019

ghost commented Mar 1, 2019

Sitemap filename can't exceed 32 characters #13937 #20044

Sitemap filename can't exceed 32 characters #13937 #20044

Conversation

irajneeshgupta commented Jan 2, 2019 • edited Loading

Description (*)

Fixed Issues (if relevant)

Contribution checklist (*)

magento-engcom-team commented Jan 2, 2019

miguelbalparda commented Jan 2, 2019

irajneeshgupta commented Jan 2, 2019 • edited Loading

irajneeshgupta commented Jan 3, 2019 • edited Loading

irajneeshgupta commented Jan 10, 2019

magento-engcom-team commented Jan 13, 2019

sivaschenko left a comment

Choose a reason for hiding this comment

sivaschenko Jan 21, 2019

Choose a reason for hiding this comment

irajneeshgupta Jan 21, 2019

Choose a reason for hiding this comment

sivaschenko Jan 21, 2019

Choose a reason for hiding this comment

irajneeshgupta Jan 21, 2019

Choose a reason for hiding this comment

sivaschenko Jan 21, 2019

Choose a reason for hiding this comment

irajneeshgupta Jan 21, 2019 • edited Loading

Choose a reason for hiding this comment

sivaschenko Jan 21, 2019

Choose a reason for hiding this comment

irajneeshgupta Jan 21, 2019

Choose a reason for hiding this comment

irajneeshgupta commented Jan 22, 2019

magento-engcom-team commented Jan 23, 2019

irajneeshgupta commented Feb 1, 2019

sivaschenko commented Feb 8, 2019

ghost commented Mar 1, 2019

irajneeshgupta commented Jan 2, 2019 •

edited

Loading

irajneeshgupta commented Jan 2, 2019 •

edited

Loading

irajneeshgupta commented Jan 3, 2019 •

edited

Loading

irajneeshgupta Jan 21, 2019 •

edited

Loading