8000 Feature/gh 11325 by ren-tham · Pull Request #11440 · SAP/spartacus · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Feature/gh 11325 #11440

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
Merged
merged 28 commits into from
Mar 22, 2021
Merged

Feature/gh 11325 #11440

merged 28 commits into from
Mar 22, 2021

Conversation

ren-tham
Copy link
Contributor
@ren-tham ren-tham commented Mar 9, 2021

No description provided.

@ren-tham ren-tham requested a review from a team March 9, 2021 18:41
@ren-tham ren-tham temporarily deployed to dev March 9, 2021 18:41 Inactive
@ren-tham ren-tham mentioned this pull request Mar 9, 2021
Closed
10 tasks
@bgambocjaviniar bgambocjaviniar temporarily deployed to dev March 10, 2021 21:46 Inactive
@bgambocjaviniar bgambocjaviniar requested a review from dydome March 10, 2021 21:46
@bgambocjaviniar bgambocjaviniar temporarily deployed to dev March 10, 2021 22:27 Inactive
@bgambocjaviniar bgambocjaviniar temporarily deployed to dev March 15, 2021 16:41 Inactive
@ren-tham ren-tham temporarily deployed to dev March 15, 2021 17:20 Inactive
dydome
dydome suggested changes Mar 16, 2021
View reviewed changes
Copy link
Contributor
@dydome dydome left a comment

Choose a reason for hiding this comment

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

In my opinion, there are a couple of tests that should be added to SavedCartListComponent:

  1. trigger loadSavedCarts on init
  2. watch onSuccess on init
  3. test for goToSavedCartDetails method
  4. test for restoreSavedCart method
  5. test for onSuccess method

All above are very similar to the test in SavedCartFormDialogComponent.

@ren-tham ren-tham temporarily deployed to dev March 16, 2021 15:54 Inactive
@ren-tham ren-tham requested a review from dydome March 16, 2021 15:56
bgambocjaviniar
bgambocjaviniar requested changes Mar 16, 2021
View reviewed changes
Copy link
Contributor
@bgambocjaviniar bgambocjaviniar left a comment

Choose a reason for hiding this comment

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

I would like you to include sorting of the dates in here as well

Basically, under feature-libs/cart/saved-cart/components/list/saved-cart-list.component.ts line 20, you do something like

savedCarts$: Observable<Cart[]> = this.savedCartService
    .getList()
    .pipe(
      map((lists) =>
        lists.sort((a, b) =>
       /// sort the saveTime property
/// latest to oldest
        )
      )
    );

@cypress
Copy link
cypress bot commented Mar 16, 2021 •
edited
Loading


Test summary

384 • 0 • 3 • 0 • Flakiness 0

Run details
Project spartacus
Status Passed
Commit 0f56b0b ℹ️
Started Mar 22, 2021 12:59 PM
Ended Mar 22, 2021 1:06 PM
Duration 06:42 💡
OS Linux Ubuntu - 18.04
Browser Electron 87

View run in Cypress Dashboard ➡️

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

ren-tham added 3 commits March 16, 2021 16:26
@ren-tham
Implemented sort for saved cart list page.
< 8000 input type="hidden" name="dropdown_direction" value="w" autocomplete="off" data-targets="batch-deferred-content.inputs" />
8f15748
@ren-tham
updated branch. Implemented sorting on saved cart listing page.
0818d95
@ren-tham
Clean up
c0b721e
@bgambocjaviniar bgambocjaviniar mentioned this pull request Mar 21, 2021
Merged
@github-actions
Copy link
Contributor

Hosting service deployment

❗ Spartacus deployment failed ( ERR Failed to execute command [application deploy] due to: Error: Deploy
Failed. Could not generate environment or presigned URL.
Response code: 500
Message: Could not deploy new application, the limit: 20 for the
tenant: spartacus has been reached
Type: internal_service_error

). Check job logs for details.)

@ren-tham ren-tham merged commit ffcd892 into epic/saved-carts Mar 22, 2021
@ren-tham ren-tham deleted the feature/GH-11325 branch March 22, 2021 13:28
@ren-tham ren-tham temporarily deployed to dev March 22, 2021 13:28 Inactive
@github-actions
Copy link
Contributor

Public API changes

✔️ Nothing changed in analyzed entry points.

⚠️ Some entry points are currently impossible to analyze.

Read more
  • @spartacus/asm/core - ERROR: "import * as ___ from ___;" is not supported yet for local files.
  • @spartacus/cart/saved-cart/core - ERROR: "import * as ___ from ___;" is not supported yet for local files.
  • @spartacus/cdc - ERROR: "import * as ___ from ___;" is not supported yet for local files.
  • @spartacus/core - ERROR: "import * as ___ from ___;" is not supported yet for local files.
  • @spartacus/organization/administration/core - ERROR: "import * as ___ from ___;" is not supported yet for local files.
  • @spartacus/organization/order-approval - ERROR: "import * as ___ from ___;" is not supported yet for local files.
  • @spartacus/product-configurator/rulebased - ERROR: "import * as ___ from ___;" is not supported yet for local files.
  • @spartacus/storefinder/core - ERROR: "import * as ___ from ___;" is not supported yet for local files.
  • @spartacus/storefront - ERROR: The expression contains an import() type, which is not yet supported by API Extractor:

💰 How to debug problems?

Read more

Problem with import() type

It happens when type is deduced by TS based on code and at the same time the deduced type is not present in the file.
In this specific case to support api-extractor it's worth to add type declaration explicitly.

Debugging steps:

  • go to the bot action logs
  • find api-extractor logs for broken library
  • check in which file and line the problems exists
  • build the library locally and check content of the file mentioned in logs (look for import()
  • add explicit type to problematic source code
  • build the library once again and verify that the import( is no longer present
  • commit and push the code with defined type

Problem with import * as ___

Api-extractor doesn't support this namespace syntax.
Check if you really need to use namespace in the library. Try to avoid namespaces when possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bgambocjaviniar bgambocjaviniar bgambocjaviniar requested changes

@dydome dydome dydome approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ren-tham @dydome @bgambocjaviniar
0