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

Refactor company collection lists #6439

Conversation

cgsunkel
Copy link
Contributor

Description of change

The company activity, contact and order tabs have been migrated to React Router.

Test instructions

The three pages mentioned above should work as normal.

Checklist

  • Has the branch been rebased to main?
  • Automated tests (Any of the following when applicable: Unit, Functional or End-to-End)
  • Manual compatibility testing (Browsers: Chrome, Firefox, Edge, Safari)

@cgsunkel cgsunkel requested a review from a team as a code owner January 18, 2024 11:30
Copy link

cypress bot commented Jan 18, 2024

Passing run #50337 ↗︎

0 26 0 0 Flakiness 0

Details:

Fixup! update prop type
Project: data-hub-frontend Commit: 5c0a479d5a
Status: Passed Duration: 02:01 💡
Started: Jan 22, 2024 12:03 PM Ended: Jan 22, 2024 12:05 PM

Review all test suite changes for PR #6439 ↗︎

@cgsunkel cgsunkel force-pushed the feature/refactor-company-collection-lists branch 2 times, most recently from 4911142 to e20972c Compare January 19, 2024 10:35
Comment on lines +130 to +131
{ url: '/companies/:companyId/hierarchies/ghq/:globalHqId/add' },
{ url: '/companies/:companyId/hierarchies/ghq/remove' },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These pages only appear very briefly while the user is updating a Global HQ, which is causing flakiness in the a11y suite.

Comment on lines +19 to +21
useEffect(() => {
document.title = `${pageTitle} - ${company.name} - Companies - DBT Data Hub`
}, [`${pageTitle} - ${company.name} - Companies - DBT Data Hub`])
Copy link
Contributor

Choose a reason for hiding this comment

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

You may want to incorporate this new component into your PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As the linked PR hasn't been merged to main yet and this PR is going into a feature-merge branch, would it be alright if I merge this now and make this change in a subsequent PR after Peter has merged his?

@cgsunkel cgsunkel force-pushed the feature-merge/company-layout-refactoring branch from 07458ad to ff633a5 Compare January 22, 2024 11:51
@cgsunkel cgsunkel force-pushed the feature/refactor-company-collection-lists branch from c015e5e to 5c0a479 Compare January 22, 2024 11:57
@cgsunkel cgsunkel merged commit 17a8f26 into feature-merge/company-layout-refactoring Jan 22, 2024
14 checks passed
@cgsunkel cgsunkel deleted the feature/refactor-company-collection-lists branch January 22, 2024 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants