flyXC icon indicating copy to clipboard operation
flyXC copied to clipboard

feat: score uploaded flight

Open flyingtof opened this issue 1 year ago • 11 comments

Score Flight Feature:

  • add "score flight" action in planner tool
  • this action is visible only when there is a flight track selected
  • the score is computed on the current selected flight
  • during the score computation, a toast is displayed
  • when score is computed,
    • the score (points,kms,speed,duration) is displayed in the planner tool,
    • the optimized route is displayed on the map
    • for triangle and out-and-return flight, closing area and FAI sectors are displayed on the map

Notable Changes

  • The worker is embedded in a Scorer class that is used in 'user made path scoring' use case and 'uploaded flight scoring'.
  • In the PathElement, the drawing of the result is launched when the score changes in the application state. Formerly, it was launched explicitly after the score computation.
  • The Score class have been changed. It is now an union of the ScoringResult and an origin flag that indicates if the scoring was performed in a 'user made path scoring' context or a 'uploaded flight scoring' context. This information is required in the PathElement class to protect the 'user made path' from overriding.

Pending points

  • Can we remove the origin flag in the Score class ?
  • Can we hide the requestId handling in the Scorer ?

TODO

  • display a cancelation dialog during long score computation

Summary by Sourcery

This pull request adds a new feature for scoring uploaded flights, displaying results in the planner and map. It also includes significant refactoring by introducing a 'Scorer' class to handle scoring logic and updating the 'PathElement' to draw results based on state changes.

  • New Features:
    • Introduced a new feature to score uploaded flights, displaying results in the planner and map, with editable results on the map.
  • Enhancements:
    • Refactored the scoring logic by embedding the worker in a new 'Scorer' class, used for both user-made path scoring and uploaded flight scoring.
    • Updated the 'PathElement' to draw results automatically when the score changes in the application state.

Summary by CodeRabbit

  • New Features

    • Introduced 'drawOptimization' and 'postScoreToHost' methods for improved path element handling.
    • Added a collapsible section and event handling for scoring tracks.
  • Refactor

    • Refactored setPath and optimize methods for enhanced path management and scoring.
    • Updated resetHandler, shouldUpdate, and other methods for better state and event handling.
  • Enhancements

    • Enhanced the planner element with new state properties and optimized score handling.
    • Improved path element responsiveness by making score a private property and updating it in stateChanged.

flyingtof avatar Jun 08 '24 08:06 flyingtof

Walkthrough

The changes enhance the functionality of the PathElement and Scorer classes within the fxc-front application. Key updates include the introduction of a new scoring mechanism, the refactoring of existing classes to streamline scoring operations, and improvements to user feedback during interactions. The modifications focus on better state management and integrating asynchronous processing for scoring requests.

Changes

File Path Change Summary
apps/fxc-front/src/app/components/2d/path-element.ts Refactored scoring logic, added RuntimeTrack, and updated methods for handling scoring results and state changes.
apps/fxc-front/src/app/logic/score/scorer.ts Removed Score class, implemented Scorer class with web worker support, and added new methods for scoring requests.
apps/fxc-front/src/app/redux/planner-slice.ts Updated score property from Score to ScoringResult in PlannerState and modified setScore reducer accordingly.
libs/optimizer/src/lib/optimizer.ts Modified ScoringResult interface to make circuit required and added a new path property for optimized paths.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant PathElement
    participant Scorer
    participant PlannerElement
    participant ReduxStore

    User ->> PathElement: Modify Path
    PathElement ->> PathElement: Update currentTrack
    PathElement ->> Scorer: score()
    Scorer ->> PathElement: return ScoringResult
    PathElement ->> PlannerElement: dispatch(handleScoreAction)
    PlannerElement ->> ReduxStore: dispatch(setScore)

Poem

In fields of code where paths traverse,
The scorer hops, no need to rehearse.
With updates swift and methods bright,
The optimized paths now take flight.
Redux states and scores align,
Enhancing routes, so fine!
🎉🐇✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar Jun 08 '24 08:06 coderabbitai[bot]

Reviewer's Guide by Sourcery

This pull request introduces a new feature for scoring uploaded flights. The implementation includes embedding the worker in a 'Scorer' class, updating the 'PathElement' to draw results when the score changes, and modifying the 'Score' class to include an 'origin' flag. The planner tool now displays a 'Score Track' action when a track is selected, and the results are editable on the map.

File-Level Changes

Files Changes
apps/fxc-front/src/app/components/2d/path-element.ts
apps/fxc-front/src/app/logic/score/scorer.ts
apps/fxc-front/src/app/components/2d/planner-element.ts
Refactored the scoring logic by introducing a 'Scorer' class, updating the 'PathElement' to handle score changes, and adding new state properties and methods in 'PlannerElement' to manage track scoring.

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

sourcery-ai[bot] avatar Jun 08 '24 08:06 sourcery-ai[bot]

Hope this PR will be more reviewer friendly than previous ones.

flyingtof avatar Jun 08 '24 08:06 flyingtof

About complexity issues, I think that we should decide together what to do. That's why the related conversation are not resolved yet.

  • State: put Score is in it's own state slice (does it make sens that it is in the planner state slice?)
  • Ui Components: create new ones (split planner-element?)
  • Other??
  • Do we postpone these refactorings in another PR?

flyingtof avatar Jun 09 '24 14:06 flyingtof

About complexity issues, I think that we should decide together what to do. That's why the related conversation are not resolved yet.

  • State: put Score is in it's own state slice (does it make sens that it is in the planner state slice?)
  • Ui Components: create new ones (split planner-element?)
  • Other??
  • Do we postpone these refactorings in another PR?

I'm going to do some refactoring when I'm changing the map lib. For now let's strive for minimal & localized changes.

vicb avatar Jun 11 '24 07:06 vicb

Refactoring and optimizations introduced regressions. I'm on investigating. I change the state of the PR to draft

flyingtof avatar Jun 12 '24 07:06 flyingtof

@flyingtof can you update the description of the PR with:

  • added features / behavior (bullet list of the changes)
  • a TODO list if there are open questions / things to figure out before this can be merged

Also it would be nice to hide the comments that have been resolved / are no more applicable (in the ... menu)

Thanks!

vicb avatar Jun 20 '24 06:06 vicb

@flyingtof can you update the description of the PR with:

  • added features / behavior (bullet list of the changes)
  • a TODO list if there are open questions / things to figure out before this can be merged

Changed PR description.

Also it would be nice to hide the comments that have been resolved / are no more applicable (in the ... menu)

Cleaned up solved comments but kept visible content related to open questions

Thanks!

YW

flyingtof avatar Jun 22 '24 12:06 flyingtof

What I would like to be changed/reverted:

When one ore more flights are uploaded, the application displays the planner tool.

The "reset" action unselects the current track and then the "Score Track" is not displayed.

Also a TODO list would help in the description to see what's left to be done before the PR is mergeable.

Thanks.

vicb avatar Jun 22 '24 13:06 vicb

What I would like to be changed/reverted:

When one ore more flights are uploaded, the application displays the planner tool.

This has been reverted a few weeks ago.

The "reset" action unselects the current track and then the "Score Track" is not displayed.

same.

Also a TODO list would help in the description to see what's left to be done before the PR is mergeable.

In PR description, added a "Pending Point" instead of a TODO list as I'm not sure that implementing the two mentioned pending points is achievable with reasonable effort or if it's worth doing.

I would like to have your insight before going ahead (or not).

flyingtof avatar Jun 24 '24 17:06 flyingtof

I've been busy working on the sounding plugin but I'll get to this PR soon. Please add a to-do to display a dialog instead of a toast so that loong running optim can be cancelled. This doesn't necessarily means we have to do it in this PR but all least we should remember to create an issue for that if we don't. Thanks!

vicb avatar Jun 28 '24 10:06 vicb