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

Vue 3 / Vuetify 3 migration for COSMOS 6 #1539

Merged
merged 105 commits into from
Nov 9, 2024
Merged

Vue 3 / Vuetify 3 migration for COSMOS 6 #1539

merged 105 commits into from
Nov 9, 2024

Conversation

ryan-pratt
Copy link
Contributor

@ryan-pratt ryan-pratt commented Sep 13, 2024

Draft PR with my changes so far

  • The application runs
  • There's an issue with routing between tools. I think we will need to change how paths are registered due to breaking changes in vue-router 4.x
  • Some of the Astro stuff has been migrated, but not everything
  • Vuetify's migration linter plugin issues have been addressed, but there are still many remaining breaking changes that need to be identified/addressed manually
    • Also, I think the linter plugin did some things wrong
  • I haven't touched styles yet
  • I haven't attempted to run tests yet

closes #75
closes #553

Copy link

codecov bot commented Sep 26, 2024

Codecov Report

Attention: Patch coverage is 85.19417% with 61 lines in your changes missing coverage. Please review.

Project coverage is 76.27%. Comparing base (420b016) to head (f245187).
Report is 106 commits behind head on main.

Files with missing lines Patch % Lines
...xtractor/src/tools/DataExtractor/DataExtractor.vue 84.61% 6 Missing ⚠️
...plorer/src/tools/BucketExplorer/BucketExplorer.vue 50.00% 5 Missing ⚠️
...cketviewer/src/tools/PacketViewer/PacketViewer.vue 77.27% 5 Missing ⚠️
...ns/packages/openc3-cosmos-tool-admin/src/router.js 50.00% 3 Missing ⚠️
...ages/openc3-cosmos-tool-cmdtlmserver/src/router.js 72.72% 3 Missing ⚠️
...-tlmviewer/src/tools/TlmViewer/NewScreenDialog.vue 76.92% 3 Missing ⚠️
...s-tool-tlmviewer/src/tools/TlmViewer/TlmViewer.vue 57.14% 3 Missing ⚠️
...cosmos-init/plugins/openc3-tool-base/src/router.js 71.42% 1 Missing and 1 partial ⚠️
...dtlmserver/src/tools/CmdTlmServer/CmdTlmServer.vue 0.00% 2 Missing ⚠️
...viewer/src/tools/DataViewer/AddComponentDialog.vue 60.00% 2 Missing ⚠️
... and 23 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1539      +/-   ##
==========================================
- Coverage   76.57%   76.27%   -0.30%     
==========================================
  Files         620      603      -17     
  Lines       47107    46119     -988     
  Branches      850      837      -13     
==========================================
- Hits        36071    35179     -892     
+ Misses      10941    10844      -97     
- Partials       95       96       +1     
Flag Coverage Δ
python 83.98% <ø> (ø)
ruby-api 48.70% <100.00%> (-0.04%) ⬇️
ruby-backend 82.04% <ø> (-0.48%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jmthomas jmthomas mentioned this pull request Oct 2, 2024
@jmthomas
Copy link
Member

jmthomas commented Oct 2, 2024

closes #553

ryan-pratt and others added 10 commits October 3, 2024 12:07
(this is largely still untested but gets me closer to where I need to be
for now)
- Changed CommandParameterEditor to align with !1471. This whole
component had to be tweaked a bunch, so I decided to knock that out too
since it made this simpler.
- Radio buttons in TopBar menus are currently not implemented. I think
we'll need to revamp how we build those menus to support it in Vuetify
3.
Skipping one test for now until TlmViewer works
@ryan-pratt ryan-pratt marked this pull request as ready for review November 4, 2024 20:50
@@ -112,8 +112,6 @@ def initialize(name, bit_offset, bit_size, data_type, endianness, array_size = n
@messages_disabled = nil
@state_colors = nil
@limits = PacketItemLimits.new
@persistence_setting = 1
@persistence_count = 0
Copy link
Member

Choose a reason for hiding this comment

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

I think these were left overs because they are also in PacketItemLimits

limits['red_high'] = limits_values[3]
limits['green_low'] = limits_values[4] if limits_values[4]
limits['green_high'] = limits_values[5] if limits_values[5]
config['limits'][limits_set] = limits
end
Copy link
Member

Choose a reason for hiding this comment

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

This logic ensures there is always a limits key but it may just be empty unless self.limits.values or self.state_colors is defined

config['limits'][limits_set] = limits
end
end
config['limits_response'] = self.limits.response.as_json(*a) if self.limits.response
Copy link
Member

Choose a reason for hiding this comment

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

This key shouldn't have been used ... it's already at config['limits']['response']

require 'openc3-enterprise/utilities/metric'
rescue LoadError
# Open Source Edition - Do nothing here
end
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if this was a future enhancement or what ... there is no openc3-enterprise/utilities/metric

Copy link
Member

Choose a reason for hiding this comment

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

This exists and should be reverted.

model = TriggerGroupModel.new(name: 'DEFAULT', scope: @scope)
model.create()
model.deploy()
end
end
Copy link
Member

Choose a reason for hiding this comment

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

Do these new ENTERPRISE conditionals require a migration?

if ENTERPRISE
# Critical Cmd Microservice
deploy_critical_cmd_microservice(gem_path, variables, @parent)
end
Copy link
Member

Choose a reason for hiding this comment

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

Do these new ENTERPRISE conditionals require a migration?

Copy link
Member

Choose a reason for hiding this comment

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

Yes removing the critical cmd microservice code requires a migration that only runs on open source.


Topic.del("#{@scope}__openc3_autonomic")
Topic.del("#{@scope}__TRIGGER__GROUP")
end
Copy link
Member

Choose a reason for hiding this comment

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

Do these new ENTERPRISE conditionals require a migration?

entries.concat(Dir.entries(folder).map { |entry| File.join(folder, entry) })
end
entries = entries.sort() # run in alphabetical order
end
Copy link
Member

Choose a reason for hiding this comment

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

We now have both base and enterprise migrations ... we'll need to test this

if File.exist?("../static/img/telemetry_viewer/widgets/#{keyword.downcase}.png")
page << "![#{keyword}](/img/telemetry_viewer/widgets/#{keyword.downcase}.png)\n\n"
end
end
Copy link
Member

Choose a reason for hiding this comment

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

This is prep for the new images in the telemetry viewer screens

entries.concat(Dir.entries(folder).map { |entry| File.join(folder, entry) })
end
entries = entries.sort() # run in alphabetical order
end
Copy link
Member

Choose a reason for hiding this comment

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

If there is a folder argument doesn't seem to be handled.

if ENTERPRISE
# Critical Cmd Microservice
deploy_critical_cmd_microservice(gem_path, variables, @parent)
end
Copy link
Member

Choose a reason for hiding this comment

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

Yes removing the critical cmd microservice code requires a migration that only runs on open source.

@@ -168,7 +166,6 @@ def states=(states)
end

@states = upcase_states
@state_colors ||= {}
Copy link
Member

Choose a reason for hiding this comment

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

This should be reverted.

require 'openc3-enterprise/utilities/metric'
rescue LoadError
# Open Source Edition - Do nothing here
end
Copy link
Member

Choose a reason for hiding this comment

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

This exists and should be reverted.

Copy link

sonarcloud bot commented Nov 9, 2024

@jmthomas jmthomas merged commit d5e1fc8 into main Nov 9, 2024
26 of 27 checks passed
@jmthomas jmthomas deleted the vue3 branch November 9, 2024 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants