Skip to content

Angular2+#532

Open
kortemik wants to merge 11 commits into
mainfrom
angular2+
Open

Angular2+#532
kortemik wants to merge 11 commits into
mainfrom
angular2+

Conversation

@kortemik

Copy link
Copy Markdown
Member

Description

Checklists

Testing

General

  • I have checked that my test files and functions have meaningful names.
  • I have checked that each test tests only a single behavior.
  • I have done happy tests.
  • I have tested only my own code.
  • I have tested at least all public methods.

Assertions

  • I have checked that my tests use assertions and not runtime overhead.
  • I have checked that my tests end in assertions.
  • I have checked that there is no comparison statements in assertions.
  • I have checked that assertions are in tests and not in helper functions.
  • I have checked that assertions for iterables are outside of for loops and both sides of the iteration blocks.
  • I have checked that assertions are not tested inside consumers.

Testing Data

  • I have tested algorithms and anything else with the possibility of unbound growth.
  • I have checked that all testing data is local and fully replaceable or reproducible or both.
  • I have checked that all test files are standalone.
  • I have checked that all test-specific fake objects and classes are in the test directory.
  • I have checked that my tests do not contain anything related to customers, infrastructure or users.
  • I have checked that my tests do not contain non-generic information.
  • I have checked that my tests do not do external requests and are not privately or publicly routable.

Statements

  • I have checked that my tests do not use throws for exceptions.
  • I have checked that my tests do not use try-catch statements.
  • I have checked that my tests do not use if-else statements.

Java

  • I have checked that my tests for Java uses JUnit library.
  • I have checked that my tests for Java uses JUnit utilities for parameters.

Other

  • I have only tested public behavior and not private implementation details.
  • I have checked that my tests are not (partially) commented out.
  • I have checked that hand-crafted variables in assertions are used accordingly.
  • I have tested Object Equality.
  • I have checked that I do not have any manual tests or I have a valid reason for them and I have explained it in the PR description.

Code Quality

  • I have checked that my code follows metrics set in Procedure: Class Metrics.
  • I have checked that my code follows metrics set in Procedure: Method Metrics.
  • I have checked that my code follows metrics set in Procedure: Object Quality.
  • I have checked that my code does not have any NULL values.
  • I have checked my code does not contain FIXME or TODO comments.

sryhanen and others added 3 commits May 19, 2026 16:12
Co-authored-by: sryhanen <samuli.ryhanen@fail.safe.net>
* Squashed Issue-469

* Issue 505/html output (#2)

* HtmlPlugin object

* HtmlFormat object

* Angular view for html plugin

* Add html format to object tree

* Small fixes

* Remove css class

---------

Co-authored-by: sryhanen <samuli.ryhanen@fail.safe.net>

* Fixed tests

---------

Co-authored-by: sryhanen <samuli.ryhanen@fail.safe.net>
* Updated testing library to compatible version with angular19+

* Added safeJson object for handling json data

* Paragraph collection object

* Refactored notebook responsibilities

* Refactored notebookCollection responsibilities

* Refactored paragraph responsibilities

* Added message object

* Removed DTO objects and added message handlers

* Refactored outputContainer responsibilities

* Refactored outputPlugin and outputFormat responsibilities

* OutputFormat implementations refactored

* OutputPlugin implementations refactored

* OutputSwitcher implementation refactored

* AngularObjectCollection responsibilities refactored

* Added more message handlers

* Removed obsolete files and refactored after removing DTOs

* Refactored angular components, separated angular logic from business logic

* Refactored legacy websocket messages to comply with basic message format

* Required changes to devserver after refactoring

* Refactored OutputContainer to hold the responsibility for active plugin

* Updated tests

---------

Co-authored-by: sryhanen <samuli.ryhanen@fail.safe.net>
@kortemik
kortemik requested a review from eemhu May 20, 2026 10:48
@sryhanen

Copy link
Copy Markdown
Contributor

One enhancement I've been thinking, would be refactor the directory structure for new code:

    • ./app/objects should contain only the business logic, i.e. the object code
    • ./app/html ( 'html' or something similar) should hold everything related for rendering the UI
    • ./app/html/angular2+ contains logic for modern angular implementations
    • ./app/html/angularJs contains logic for legacyAjs implementations required to keep

@kortemik @eemhu
Any thoughts on this, and should it be implemented?

* Directive for rendering outputView programmatically

* Refactored OutputView rendering from outputContainer

* Modern input signals seem not to be entirely supported in a hybrid application, replaced input signal with traditional input decorator

* Added fakePlugin for testing

* Renamed input name

* Fixed and refactored test

* Added test file for pluginView

* Add license

---------

Co-authored-by: sryhanen <samuli.ryhanen@fail.safe.net>
@sryhanen

Copy link
Copy Markdown
Contributor

Started implementing #535 for this

@eemhu

eemhu commented May 26, 2026

Copy link
Copy Markdown

One enhancement I've been thinking, would be refactor the directory structure for new code:

* * ` ./app/objects`  should contain only the business logic, i.e. the object code

* * ` ./app/html` ( 'html' or something similar) should hold everything related for rendering the UI

* * ` ./app/html/angular2+` contains logic for modern angular implementations

* * ` ./app/html/angularJs` contains logic for legacyAjs implementations required to keep

@kortemik @eemhu Any thoughts on this, and should it be implemented?

sounds like a decent idea, perhaps html could be named ui though

@sryhanen

Copy link
Copy Markdown
Contributor

One enhancement I've been thinking, would be refactor the directory structure for new code:

* * ` ./app/objects`  should contain only the business logic, i.e. the object code

* * ` ./app/html` ( 'html' or something similar) should hold everything related for rendering the UI

* * ` ./app/html/angular2+` contains logic for modern angular implementations

* * ` ./app/html/angularJs` contains logic for legacyAjs implementations required to keep

@kortemik @eemhu Any thoughts on this, and should it be implemented?

sounds like a decent idea, perhaps html could be named ui though

Started working on this:
#548

@sryhanen

Copy link
Copy Markdown
Contributor

Started implementing #535 for this

#547

sryhanen and others added 2 commits May 27, 2026 10:50
* NotebookCollection integration test, and fixes for notebookCollection

* WritableSignalArrayAsPushValue object for handling values with list type

* Renamed unit tests with .unit prefix

* ParagraphCollection object

* Refactored paragraph rendering to ParagraphCollectionView

* NotebookView integration spec

* ParagraphCollectionView integration tests

* ParagraphView integration tests

* OutputContainerView integration spec

* Renamed spec files

---------

Co-authored-by: sryhanen <samuli.ryhanen@fail.safe.net>
Co-authored-by: sryhanen <samuli.ryhanen@fail.safe.net>
@sryhanen

Copy link
Copy Markdown
Contributor

Problems detected during qa testing fixed in pr #555

sryhanen and others added 2 commits May 29, 2026 13:02
* Fixed angular output format parsing

* Render only current notebook in collection if id matches

* ad hoc fix for broken output data format

* Fixed Ajs template input name

* Link paragraphCollection to angularObject collection so that on request paragraphCollection can decorate requests

* Text plugin needs to add class value to html element

* Pass responses to angularObjectCollection

* Log error instead of warning

* ParagraphCollection refactored as stubable, due notebook being partial initially

* Updated notebook spec

* Added tests for paragraphCollection

* Re-use variable

---------

Co-authored-by: sryhanen <samuli.ryhanen@fail.safe.net>
* Refactored paragraphCollection as property of paragraph, because paragraph is responsible for filtering messages that paragraphCollection receives

* Logic for handling paragraphId was missing from angular object, refactored object to encapsulate paragraphId

---------

Co-authored-by: sryhanen <samuli.ryhanen@fail.safe.net>
* a licensee so wish it.
*/
/*
* Licensed under the Apache License, Version 2.0 (the 'License');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

the file appears to have two licenses

spyOn(scope, 'updateParagraphObjectWhenUpdated');
spyOn(scope, 'updateAllScopeTexts');
scope.updateParagraph(paragraphExampleMock.paragraph, paragraphClearMock.paragraph, emptyCallback);
scope.updateParagraph(paragraphExampleMock.paragraph, paragraphClearMock.paragraph);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

wasn't the paragraph object removed? from paragraphExampleMock

* Refactored encapsulate properties of angularObject

* Refactored angularObjectCollection initialization to paragraph object

* Changed test description

---------

Co-authored-by: sryhanen <samuli.ryhanen@fail.safe.net>
}

value(): unknown {
return this._angularObjectData['object'];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

any reason to not use the SafeJson object? this way the non-validated object could be removed as a field variable and only have the SafeJson variant.

Perhaps SafeJson could be renamed as a ValidatedObject or such.

export class AngularObjectCollectionImpl implements AngularObjectCollection {
private readonly _channel: Channel;
private readonly _angularObjects: AngularObject[];
private readonly _pushValues: PushValue<AngularObject[]>[];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't think I fully understand the purpose of the PushValue object, could you explain a bit?

*/

export interface MessageWithAuthenticationInfo {
print(): {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

would message() be more descriptive here

ticket: '',
roles: '',
};
if(!this._authentication.isStub()){

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I would leave authenticationInfo undefined and define it in else-block if authentication.isStub=false.

expect(angularObjectCollection).toBeInstanceOf(AngularObjectCollectionImpl);
});

it('Should have empty collection of angular objects', () => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

non-empty test?

outputContainer.request(request);
expect(channelSpy).toHaveBeenCalledExactlyOnceWith(request);
});
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should we also test response here

});

describe('Plugin formatting', () => {
const plugingData = {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

typo

* Added missing webpackplugin for loading angular templates properly

* Add AngularWebpackPlugin to production webpack config as well

* This causes now type error

* Now input signals work

* Exclude unused files

* POC solution for refactor

* Fix recursion pattern from Render and RenderNode

* poc version 2

* use signals for component inputs as well

* Recursive component rendering

* Stash

* Poc works now

* small fix

* Refactored directories

* ComponentView object

* ParagraphCollectionRefactored

* OutputContainer refactored

* Output render works almost now, expect inputs are not updated since they are not wrapped to signals

* Fixed signal read for RecursiveComponentDraw

* Updated stub outputPlugin

* webAppRoot needs to print signal and update it each response

* Can use computed signal here instead of effect

* Fix bug where adding paragraph caused disorientation of the paragraph mapping

* This is not part of the objects responsibilities

* Started refactoring OutputFormats

* uPlot refactored

* Renamed method

* textOutput

* AngularFormat

* Deleted unused files

* Removed WebAppComponentRegistry since it caused unnecessary complexity

* Refactored outputPlugins, removed unused ones

* Deleted unused file

* Started refactoring outputSwitcher

* Switcher refactored

* Ad hoc fix for uPlotView change detection

* Refactored outputFormat validation and conditional switching to separate object

* Override inherited exclude rule for spec config

* Fixed tests

* Fixed tests

* Fixed tests

* Fixed tests

* Fixed tests

* Fixed tests

* Fixed tests

* Fixed tests

* Subtyped OutputFormat objects

* Fixed tests

* Fixed tests

* Added test

* Fixed tests

* Added test

* Refactored NotebookCollection

* Updated tests

* ParagraphCollection is no longer stubable

* Added test

* Implemented generic filter object

* Refactored paragraphOutputMessage as stubable

* ParagraphDataAsOutputMessage object

* Initialize decorator paragraph collection

* Deleted push value objects

* Patched tests

* Core idea of the response refactor

* Deleted obsolete objects

* Decorator ResponseRegister objects

* Refactor to use ResponseRegister

* ParagraphMessage object

* ParagraphAddedMessage

* ParagraphRemovedMessage object

* Refactored ParagraphCollection

* Added RequestRegister object

* Refactored paragraphCollection

* Added unit test

* fixed RequestRegister implementation

* RequestRegisterWithPropertyDecorator object

* Fixed implementations

* Refactored interpreterErrorListener

* Fixed tests

* fixed test

* removed signal

* fixed tests

* fixed tests

* fixed tests

* Refactored ParagraphOutputMessage

* Refactored ParagraphOutputMessage

* Fixed test

* Refactored OutputFormat SwitcherButtons property

* Fixed tests

* Added test

* Fixed test

* Fixed tests

* Added and fixed tests

* Fixed test

* Added test

* Fixed implementation and test

* Added test

* Added test

* Added testing library plugin for asserting changes for components who rely on css. Bootstrap5 modal for instance is such component.

* Added test file

* Added host class

* Added test

* Added tests

* Added tests

* Added tests

* Added tests

* Added test

* Added test

* Karma unit tests require additional webpack config that uses ts-loader

---------

Co-authored-by: sryhanen <samuli.ryhanen@fail.safe.net>
@kortemik
kortemik requested a review from eemhu June 26, 2026 07:38
…ion (#581)

Co-authored-by: sryhanen <samuli.ryhanen@fail.safe.net>

it('Should have notebookIndices', () => {
expect(notesInfoMessage.notebookIndices().size).toEqual(2);
expect(notesInfoMessage.notebookIndices().get('notebook1')).toBeDefined();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

would assert the actual indices


private validateType():void{
if(this._type !== this._message.operation()){
throw new RangeError(`Message type is not "${this._type}".`);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

would be worth to have the expected and actual message type in the error message

const paragraphOutputMessage = new ParagraphOutputMessageImpl(message);
if(paragraphOutputMessage.type() !== OutputType.angular){
this._componentView.set(this._componentViewStub);
return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

use else block instead of early return

if(paragraphOutputMessage.type() !== OutputType.dataTables){
this._componentView.set(this._componentViewStub);
this._plugin.set(this._pluginStub);
return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

replace early return with else block

const paragraphOutputMessage = new ParagraphOutputMessageImpl(message);
if(paragraphOutputMessage.type() !== OutputType.html) {
this._componentView.set(this._componentViewStub);
return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

replace with else block

import {RgbColor} from './rgbColor';

export class RgbColorImpl implements RgbColor {
private readonly _red:number;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could be improved with RgbaColor decorator. But perhaps leave that for another PR.

RgbColor(red, green, blue)
RgbaColor(RgbColor, opacity)

Comment on lines +66 to +73
const uPlotOutputOptions = {
labels:safeOutputOptions.getProperty<string[]>('labels', 'object'),
series:safeOutputOptions.getProperty<string[]>('series', 'object'),
xAxisLabel:safeOutputOptions.getProperty<string>('xAxisLabel', 'string'),
graphType: safeOutputOptions.getProperty<string>('graphType', 'string'),
};
let uPlotOptions:uPlot.Options;
const basicOptions = new BasicOptionsImpl(uPlotOutputOptions);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I feel like this could be part of the BasicOptions object

import {ComponentView} from '../../rendering/componentView/componentView';
import {OutputFormatsWithValidatedOutputSwitch} from './outputFormatsWithValidatedOutputSwitch';

export class OutputFormatsWithValidatedOutputSwitchImpl implements OutputFormatsWithValidatedOutputSwitch {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not really clear on the purpose of this object. At least the name could use a fix

import {ParagraphOutputMessageImpl} from '../../message/paragraphOutputMessage/paragraphOutputMessageImpl';
import {MessageImpl} from '../../message/messageImpl';

export class ParagraphDataAsOutputMessageImpl implements ParagraphDataAsOutputMessage {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ParagraphOutputMessageFactory


const paragraphDataAsOutputMessage = new ParagraphDataAsOutputMessageImpl(paragraph);
const paragraphOutputMessage = paragraphDataAsOutputMessage.paragraphOutputMessage();
if(!paragraphOutputMessage.isStub()){

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

way to clean up ctor?

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