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.

Sign up for GitHub

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

Jump to bottom

Refactor Hash Property Parsing for DSM Variants #334

Merged
merged 7 commits into from
Feb 12, 2019
Merged

Refactor Hash Property Parsing for DSM Variants #334

merged 7 commits into from
Feb 12, 2019

Conversation

pr1sm
Copy link
Collaborator

@pr1sm pr1sm commented Feb 11, 2019 •
edited by walmat
Loading

Changes

  • Refactored DsmParser to accept optionally parsing for hash properties on the initial/product pages
    • Default implementation does nothing
  • Added DsmUsParser to handle parsing for a hash property when using the DSM US site
    • This contains the site specific code that was previously in DsmParser before it was refactored
  • Added DsmUkParser to handle parsing for a hash property when using the DSM UK site
    • Parses for a custom.js source and fetches the content, then parses for hash property
    • If the initial pages contains the hash, the product pages skip hash parsing, prevent an extra, unnecessary request
  • Updated specialParser to allow for asynchronous page parsing
    • Allows DsmUkParser to fetch the custom.js file for further parsing
  • Fix lint warnings in task-runner project

Checks

  • CI passes
  • Coverage (<2%∆)
  • Manual Checks
    • Run a task on DSM JP (if it is still up, otherwise use Mock Server)
      • Ensure base DsmParser implementation works and finds product correctly
    • Run a task on DSM US (if it is still up, otherwise use Mock Server)
      • Ensure the correct hash property is parsed on the product page
      • Ensure parser finds the product correctly
    • Run a task on DSM UK -- Use Mock server to test this
      • Ensure the correct hash property is parsed on the initial page
        • Ensure the product page skips the hash parsing
      • Adjust the custom.js script tag so it points to a broken link and ensure the product page is used to parse the hash
      • Remove custom.js script tag from the response of the initial page and ensure the product page is used to parse the hash

Using Mock Server to Test

The Mock server can be used, but there are a couple of setup steps that need to be performed before testing:

  1. Adjust the siteOptions.js file to set the Mock Server's special attribute
  2. Adjust the .env.dev file to set the NEBULA_RUNNER_MOCK_SPECIAL_PARSER env var to the correct site
  3. Load the initial page and product page routes in the Mock Server

fixes #314

@pr1sm
Refactor DsmParser
a881585 
This commit refactors DsmParser to offer extensibility when parsing for
site/product specific hash properties. By default no parsing is done, but
subclasses of DsmParser can now provide an implementation to have this
parsing done if necessary on the initial page, product pages, or both

Additionally, the file is moved to a new folder where all dsm parser
variants can live.
@pr1sm
Implement DsmUsParser
499d6b4 
This commit implements a specific DsmParser variant for the US Site. This
includes parsing a product page for a specific hash property that is
within a binary encoded string in a script attached to the product page.
@pr1sm
Implement DsmUkParser
b7147d6 
This commit implements a specific DsmParser variant for the UK Site. This
includes parsing for a custom.js sourced script in both the initial page
and product pages head element. The script is then fetched and its contents
are parsed to find the included hash property. If the hash property is
found on the initial page, the parsing for the hash property on the product
page is skipped -- this prevents an unnecessary additional request.
@pr1sm
Update Special Parser Selections
d7d4ce4 
This commit updates the getSpecialParser method to include the
refactored DsmParser as well as the new variants for specific sites.

The specialParser is also updated slightly to allow for asynchronous
implementations when parsing the initial/product pages. This allows the
DsmUkParser to function properly since it has to fetch the custom.js
file for parsing the hash property.
@pr1sm
Fix Task Runner Lints
dc9711a 
This commit fixes some small lints to remove all lint errors/warnings
in the task runner package.
@pr1sm pr1sm added type:enhancement New feature or request area:task-runner Related to Nebula's Task Runner package focus:parsing things involving site parsing (for task runner) labels Feb 11, 2019
@pr1sm pr1sm added this to the Beta 6 Release milestone Feb 11, 2019
@pr1sm pr1sm requested a review from walmat February 11, 2019 22:56
walmat
walmat previously requested changes Feb 11, 2019
View reviewed changes
Copy link
Owner

@walmat walmat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job man! Love the splitting of the classes 🔥 Just one small typo change, otherwise, really good job seriously!

packages/task-runner/src/shopify/classes/parsers/dsm/dsmUkParser.js Outdated Show resolved Hide resolved
@walmat @pr1sm
Fix Typo in Logs
cdeef5d 
Co-Authored-By: pr1sm <dhanwada.dev@gmail.com>
@pr1sm pr1sm dismissed walmat’s stale review February 12, 2019 00:26

requested changes have been made

@pr1sm
Copy link
Collaborator Author

pr1sm commented Feb 12, 2019

@walmat I think this is ready for testing now

@pr1sm
Finish Writing Doc Comment
231e884 
This commit finishes an incomplete doc comment.
@walmat
Copy link
Owner

walmat commented Feb 12, 2019

@walmat I think this is ready for testing now

Going through tests now. Working on DSM UK now

@walmat
Copy link
Owner

walmat commented Feb 12, 2019

All manual tests passed. 🔥

@walmat
Copy link
Owner

walmat commented Feb 12, 2019

merging...

@walmat walmat merged commit 99140e5 into b1.0.0-beta.6 Feb 12, 2019
@walmat walmat deleted the issue_314 branch February 12, 2019 04:54
@walmat walmat mentioned this pull request Feb 12, 2019
Closed
@pr1sm pr1sm mentioned this pull request Feb 23, 2019
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@walmat walmat walmat approved these changes

Assignees
No one assigned
Labels
area:task-runner Related to Nebula's Task Runner package focus:parsing things involving site parsing (for task runner) type:enhancement New feature or request
Projects
None yet
Milestone
Beta 6 Release
Development

Successfully merging this pull request may close these issues.
2 participants
@pr1sm @walmat