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

Migrate Baw Client Routes #780

Merged
merged 3 commits into from
Feb 11, 2021
Merged

Migrate Baw Client Routes #780

merged 3 commits into from
Feb 11, 2021

Conversation

Allcharles
Copy link
Contributor

@Allcharles Allcharles commented Jan 20, 2021

Migration of Baw Client Routes

Create all of the routing required to handle displaying the baw client pages. This makes use of the BawClientComponent created in the parent PR.

Changes

Problems

Visual Changes

Listen Page (/listen)

image

Play Page (/listen/:audioRecordingId)

image

Library Page (/library)

image

Annotation Page (/library/:audioRecordingId/audio_events/:audioEventId)

image

Visualize Page (/visualize)

image

Audio Analysis

List Page (/audio_analysis)

image

New Page (/audio_analysis/new)

image

Details Page (/audio_analysis/:analysisJobId)

image

Results Page (/audio_analysis/:analysisJobId/results)

image

Final Checklist

  • Assign reviewers if you have permission
  • Assign labels if you have permission
  • Link issues related to PR
  • Ensure project linter is not producing any warnings (npm run lint)
  • Ensure build is passing on all browsers (npm run test:all)
  • Ensure CI build is passing
  • Ensure docker container is passing (docs)

@Allcharles Allcharles added enhancement New feature or request ui feature A new feature/page to add to the project labels Jan 20, 2021
@Allcharles Allcharles marked this pull request as draft January 20, 2021 04:45
@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 20, 2021

Size Change: +4.22 kB (0%)

Total Size: 1.17 MB

Filename Size Change
dist/workbench-client/browser/assets/environment.json 401 B -30 B (-7%)
dist/workbench-client/browser/index.html 1.04 kB -1 B (0%)
dist/workbench-client/browser/main-es2015.********************.js 530 kB +1.89 kB (0%)
dist/workbench-client/browser/main-es5.********************.js 554 kB +2.37 kB (0%)
ℹ️ View Unchanged
Filename Size Change
dist/workbench-client/browser/manifest.json 150 B 0 B
dist/workbench-client/browser/polyfills-es2015.********************.js 12.6 kB 0 B
dist/workbench-client/browser/polyfills-es5.********************.js 46.1 kB 0 B
dist/workbench-client/browser/runtime-es2015.********************.js 719 B 0 B
dist/workbench-client/browser/runtime-es5.********************.js 719 B 0 B
dist/workbench-client/browser/styles.********************.css 27.2 kB 0 B

compressed-size-action

@Allcharles Allcharles added blocking/blocked Pull requests or issues that are blocked by something parent pr Pull request with children branches. Do not rebase/squash merge. labels Feb 8, 2021
@Allcharles Allcharles marked this pull request as ready for review February 9, 2021 01:13
@Allcharles Allcharles requested a review from atruskie February 9, 2021 01:13
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Member

@atruskie atruskie left a comment

Choose a reason for hiding this comment

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

I'd say a lot of these non-CRUD pages don't fit well into the list/details nomenclature we've used for CRUD pages. Listen, visualise are more composite pages that tie more than one basic resource together. Not important enough to change now, might be worth considering later.

There are several places with TODOs for malformed URLs (good). Just add issue link to each so they're findable using the same pattern.

Great work, minor changes suggested

src/app/components/sites/site/site.component.ts Outdated Show resolved Hide resolved
src/app/components/sites/site/site.component.ts Outdated Show resolved Hide resolved
src/app/components/visualize/visualize.menus.ts Outdated Show resolved Hide resolved
src/app/models/AudioEvent.ts Outdated Show resolved Hide resolved
src/app/models/Bookmark.ts Outdated Show resolved Hide resolved
src/app/services/baw-api/baw-api.service.ts Outdated Show resolved Hide resolved
src/app/services/baw-api/baw-api.service.ts Outdated Show resolved Hide resolved
@Allcharles Allcharles requested a review from atruskie February 10, 2021 04:00
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Member

@atruskie atruskie left a comment

Choose a reason for hiding this comment

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

💯

Create all of the routing required to handle displaying the baw client pages. This makes use of the `BawClientComponent` created in #727.
Build baw client in github actions, this allows the unit tests to validate that the baw client has been correctly integrated into the project.
* Cleanup app styling to create full height components

- Cleanup CSS styling for the app component so that the default router outlet component will have access to the full height of the page
- Created basic audio analysis page

* Working example

Couple bugs to fix
- AngularJS links are not reloading the parent page
- IFrame does not resize to content which causes the scrollbar to be in the wrong location

* Created new audio analysis page

* Cleanup iframe module

- Disabled broken tests
- Renamed IFrameComponent to BawClientComponent
- Isolated BawClientComponent into a module
- Created placeholder unit tests
- Updated interceptor to include cookies in requests

* Implemented BawClientComponent unit tests

* Fixed broken tests and rebase conflicts

* Fixed broken unit tests

Discovered bug in how secondary menu unit tests were saving/restoring the defaultMenu global variable (converted Lists to arrays, but not back)

* Fix site/region details tag links

* Working login request

* Update security service tests

* Cleanup interceptor tests

* Fix rebase issues

* Cleanup api request responseType's

This should make things a lot more clear for when the interceptor should set accept/content-type headers to requests

* Cleanup review comments
@Allcharles Allcharles merged commit 3231cd7 into iframes Feb 11, 2021
@Allcharles Allcharles deleted the initial-migration branch February 11, 2021 01:40
Allcharles added a commit that referenced this pull request Feb 11, 2021
Create all of the routing required to handle displaying the baw client pages. This makes use of the `BawClientComponent` created in #727.
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

Unit Test Results

         7 files  +         6           7 suites  +6   7m 34s ⏱️ + 6m 49s
15 260 tests +13 131  14 588 ✔️ +12 557  672 💤 +575  0 ❌  - 1 

Results for commit 3231cd7. ± Comparison against base commit 9d9e2a2.

@Allcharles Allcharles linked an issue Apr 16, 2021 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocking/blocked Pull requests or issues that are blocked by something enhancement New feature or request parent pr Pull request with children branches. Do not rebase/squash merge. ui feature A new feature/page to add to the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Merge with AngularJS Client
2 participants