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

GROUP-33 Display Available Groups #4

Merged
merged 1 commit into from
Nov 25, 2023
Merged

Conversation

makmn1
Copy link
Member

@makmn1 makmn1 commented Nov 25, 2023

Description of Changes

There were a lot of changes that needed to take place in order to accomplish this issue. Specifically, the following:

  • Fleshed out the management of groups into separate services to be injected into the GroupBoardComponent
  • Modularized GroupBoardComponent into several new components for reusability and maintainability
  • Created a service to handle state transitions for seamless transitions of component state to enhance the user experience
  • Added animations via Angular Animations. Implemented a custom animation service for GroupBoardComponent due to limitations of Angular Animations
  • Added configuration via ConfigService which gets configuration from a static asset
  • Removed Cypress and reverted to creating tests using traditional Angular testing tools and libraries
  • Removed E2E tests
  • Set up robust network services for HTTP and RSocket connections by gracefully recovering from errors and retrying connections
  • Revised existing and added new tests for all services and components

Below is a detailed documentation of the changes made.

RSocket Integration

To better separate the concerns of creating an RSocket connection, as well as to better unit test the logic setting up the RSocket connection, the rsocket-related logic is split into the following services:

  • RsocketService: Creates the RSocket connection
  • RsocketConnectorService: Constructs the details necessary for creating an RSocket connection
  • RsocketMetadataService: Provides the metadata necessary for authenticating and communicating with the GroupHQ RSocket server
  • RsocketPublicUpdateStreamService: Uses an existing RSocket connection to request a stream of PublicEvent objects
  • BufferPolyfill: To be injected by any class with an RSocket dependency. This ensures that the keyword Buffer is assigned as a global object to the Buffer from the buffer dependency. Unfortunately, the RSocket dependencies rely on the existence of a global Buffer object as they were initially designed to be run in a Node environment. This issue was discussed here.

Users are expected to inject RsocketPublicUpdateStreamService which will itself inject the remaining dependencies to construct an RSocket connection to the server. If successful, the application will start to receive a stream of PublicEvent objects from the server which the user can listen to be subscribing to the observable that the service provides.

GroupBoardComponent

This component is shown to user when they visit the home page. It consists of several dependencies, each of which was carefully considered.

RSocket Public Updates Stream

Subscribes to an RxJS observable for when the public updates stream is ready. If so, it delegates received event objects to be handled by the Group Manager service. Additionally, certain component state flags relating to synchronization are set based on if the stream is active or not. This conditionally triggers the appearance and text content of the new <app-sync-banner> component.

Group Manager Service

Handles group-related events by altering the client's group data based on event data. The component subscribes to an observable in this service which sends out a GroupUpdate object containing a callback to execute the change as well as additional info. Based on the information from this object, the component calls the necessary animation function to animate the group card elements using FlipService.

Flip Service

F.L.I.P. (first, last, invert, play) is an animation technique for animating elements, specifically from one position to another. It does this by having a reference to the card elements displayed on screen. Angular updates these elements as change detection takes place. For every change the component needs to make (e.g. adding/removing a group, sorting), it calls the animate function. This function:

  1. records the first positions of the elements
  2. executes the callback given (e.g. such as adding a group),
  3. performs change detection if a ChangeDetectorRef instance is passed
  4. records the last positions of the elements
  5. inverts the elements. In other words, sets each element's transform style to move it to its original position
  6. plays animation. In other words, animates the elements to their original position (by setting transform to none and animating that).

Steps 5 and 6 are accomplished using Angular's AnimationBuilder. If a group Id argument is passed to the animate function, then a special removal animation is created. Removal animations need to be treated differently since the element needs to be removed after it's removal animation finishes. However, this prevents other elements from reordering to their new positions properly since the element that's being removed is still considered to be taking up space in the document flow. To address this, elements that are being removed are also set to have a style of position: absolute to remove them from the document flow, allowing other elements to obtain their new positions instantly. The service also keeps track of element's currently animating, and if an element needs to move to another position while animating, it animates from it's current position in the animation.

If this all sounds complicated, it's because it is! I would have hoped that the above could be accomplished using Angular Animations, but unfortunately, this is not possible due to the complexity of the requirements for the desired animation here. Angular animations does a good job for the following cases:

  • Animating new items coming in
  • Animating items leaving
  • Animating items moving to their new position

However, weird bugs start to occur when given the ability to sort and reorder items (even with the recommended trackBy function to help Angular keep track of items):

  • The first item does not animated to it's new position on the initial sort, but does for subsequent sorts
  • New items added to the list experience the same issue

And this problem isn't specific to this problem. In fact, the first and earliest report I could find on this issue is over 6 years old, and it's still marked open! Due to this, I decided to opt for a custom approach. While there are a few kinks to be ironed out (specifically, sometimes animations finish instantly in very specific scenarios), it works well enough now.

Thankfully, all other animations work well with Angular Animations in the project so far.

State Transition Service

One annoying issue was trying to gracefully switch between states for the component:

  • Loading state if the component is loading groups
  • Ready state once groups have been loaded
  • Error state if groups could not be loaded
    To "gracefully switch" in this context also means to give a minimum time for a state to appear to the user (e.g. 2 seconds) so that the user understands what's happening.

To accomplish this, the State Transition service allows the queuing up of states or the ability to instantly transition to a new state (which causes the queue of states to be cleared). The implementation is satisfactory, but is likely to be removed in the next update. Since Angular v17 released about two weeks ago as of writing, one of its new features is deferrable views, which provides similar functionality to the State Transition service. Instead, it's logic is embedded in the template. The next update plans to add this functionality, rendering the State Transition service obsolete, and thus removed.

Retry Services

To help the application recover from errors gracefully, two retry services were introduced: RetryDefaultService and RetryForeverConstantService. Both can be configured through configuration properties, and both allow observables to be wrapped with retry logic. The RetryForeverConstantService is used quietly in the background by RsocketService to retry connecting to the Rsocket server every X seconds (based on the configured property). The RetryDefaultService is used by the GroupBoardComponent to retry the load groups request with exponential backoff. This service also provided an observable provided the next retry time. This information is used by GroupBoardComponent to show the user when the next retry time will occur. Users are also given the option to retry immediately, resulting in a new load groups attempt.

Note that RSocket service also has it's own retry capabilities for retrying a connection after it has been lost. While the retry operator could handle this if the RSocket library integrated with RxJS and supported retry connections, this is not the case. Instead, the RSocket library uses a promise-based system, which would complete the observable after the promise resolves (the promise is converted into an observable). Therefore, in these cases, manually re-subscribing to the observable is needed.

Identification Service

A very simple class used by HTTP and RSocket requests to provide the server with a unique Id representing the user. It's saved to local storage so that the user's identity is saved on page reloads and future sessions. It's not meant to persist forever, but instead long enough to be convenient for the user in the short time they're expected to try out the demo.

Config Service

To allow the application more flexibility, especially in different environments, support for configuration properties were added. Configuration properties should be defined in the src/config/config.json file. When a new configuration property is added to this file, then the type information for the configuration file should be updated at src/app/config/config.ts. To allow services to access properties through ConfigService, accessor methods should be added to src/app/config/config.service.ts.

The src/config directory is defined to be a build asset in the angular.json file, so it will be included as-is in the application's build, allowing the properties to be modified without having to rebuild the application.

New Approach to Testing

In previous updates, tests were handled through the Cypress framework. However, after a lot of research into testing, I decided it's better to not use Cypress for the following reasons:

Poor Angular Integration

To work with Cypress, I had to abandon Jasmine and instead use the testing libraries used by Cypress: Chai and Sinon. This was due to the type conflicts that the Cypress library introduced into the project. This issue was bought up to the Cypress team over three years ago, and it still remains open with no end in sight.

Slow. Very slow.

Specifically, I'm talking about component testing, which is what replaced my Karma w/ Jasmine workflow. Instead of running Component DOM tests and service tests using Karma, I decided to adopt component testing. And the two are mutually exclusive, which does make sense since test files need to be bootstrapped with the cy.mount command and renamed to have the cy.ts suffix.

While component testing did seem attractive due to being able to see your component while testing using a simpler API, it quickly became gimmicky. I was more interested in my logic passing than if my component styles were set right. Especially given the time to run. To run 12 Cypress component tests, it takes my computer around 45 seconds to a minute. Compare that with the now 169 unit and component tests we have running with Karma, which consistently take one second to run. It would probably be faster if I got rid of the debug logging.

Needless Complexity For Little Benefit

One of the best things about Cypress is it's seemingly simple API, but even that comes with a learning curve and different set of idioms to learn and overcome. Coupled with the very difficult learning curve of testing Angular applications, this makes testing a slog for newcomers. Especially considering that the learning curve for testing Angular applications doesn't go away when component testing with Cypress, it didn't make sense to add complexity to use a simpler syntax.

While it was nice to have a browser to run tests in, I later discovered the same thing can be done with the Karma test runner in debug mode (by placing breakpoints at certain parts of the application). Though this does not apply styles from stylesheets, issues in tests rarely result from CSS (unless your testing styles?).

Verdict

All those issues were enough to make the decision to fallback to traditional Angular testing. And I got to say, it feels great to be able to run tests up to 60 times per minutes instead of once.

As for Cypress E2E testing, I've decided to do E2E testing outside of the Angular application and in a separate application. The main reasons being:

  • Isolated application, eliminates the need to worry about integrating E2E testing suite with the Angular development environment.
  • Easier to integrate into a testing environment to independently scale in a Continuous Testing Server
  • Promotes black-box testing, which is how E2E testing is traditionally done

I've been convinced to try out E2E automated testing using Ruby with RSpec and Selenium based on articles by Zhimin Zhan. That's coming in the very near future--hopefully.

Additional Info / Concerns

  1. The initial build size is large, at around 1.06 megabytes. About 90% of this is from JavaScript. Initially, I thought it was due to the Buffer and RSocket dependencies being included as CommonJS modules which cannot be tree-shaked when building the application. But even commenting out those files only bought the total size down to around 930 KB. It's not clear yet what size would work for most users, but what is clear is that we need to take advantage of lazy and dynamic loading features as the application grows with new feature modules.
  2. Unfortunately, the RSocket libraries aren't as maintained as they should be. The library is still maturing, indicated by its alpha version tag. Additionally, the documentation is...well, it doesn't exist. Correction, it does exist, but it's outdated. The project hasn't received many updates over the past year, and there are very few active maintainers. Coupled with the fact that a final non-alpha v.1.0.0 should've released in Q2 of this year, it's not reassuring. Nevertheless, the benefits of using RSocket probably outweigh these cons. While there is no documentation, the GitHub repository does host a set of examples for the different packages. The current RSocket implementation in this issue is heavily inspired by these examples.

@makmn1 makmn1 self-assigned this Nov 25, 2023
@makmn1 makmn1 merged commit 47fdb19 into main Nov 25, 2023
2 checks passed
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.

None yet

1 participant