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.

Sign up for GitHub

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

Jump to bottom

Remove linking references #3614

Merged
merged 2 commits into from
Dec 29, 2020
Merged

Remove linking references #3614

merged 2 commits into from
Dec 29, 2020

Conversation

asmulko
Copy link
Contributor

@asmulko asmulko commented Dec 23, 2020

Summary

Closes #3542

Changes

  • Removed linking from create application
  • Removed linking from side menu
  • Removed linking info from application overview
  • Updated uplink/downlink not to depend on linking

Before:
before_1
After:
after_1

Before:
before_2
After:
after_2

Before:
before_3
After:
after_3

Testing

Manually tested

Notes for Reviewers

  • I would appreciate if someone could test this locally to see if nothing is broken, I could've missed something

Checklist

  • Scope: The referenced issue is addressed, there are no unrelated changes.
  • Compatibility: The changes are backwards compatible with existing API, storage, configuration and CLI, according to the compatibility commitments in README.md for the chosen target branch.
  • Documentation: Relevant documentation is added or updated.
  • Changelog: Significant features, behavior changes, deprecations and fixes are added to CHANGELOG.md.
  • Commits: Commit messages follow guidelines in CONTRIBUTING.md, there are no fixup commits left.

@asmulko asmulko self-assigned this Dec 23, 2020
@asmulko asmulko added the c/console This is related to the Console label Dec 23, 2020
@asmulko asmulko added this to the December 2020 milestone Dec 23, 2020
@github-actions github-actions bot added the ui/web This is related to a web interface label Dec 23, 2020
bafonins
bafonins suggested changes Dec 24, 2020
View reviewed changes
pkg/webui/console/containers/application-title-section/application-title-section.js
@@ -25,8 +25,6 @@ import EntityTitleSection from '@console/components/entity-title-section'
import sharedMessages from '@ttn-lw/lib/shared-messages'
import PropTypes from '@ttn-lw/lib/prop-types'

import ApplicationStatus from './status'
Copy link
Contributor

Choose a reason for hiding this comment

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

You can then also remove the status folder. Or can we still fetch up/down counts and last received timestamp from ApplicationLinkStats ? cc @adriansmares

Copy link
Contributor

Choose a reason for hiding this comment

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

ApplicationLinkStats is deprecated. There are no plans in bringing it back at the moment.

pkg/webui/console/containers/application-title-section/application-title-section.js
@@ -110,22 +101,15 @@ ApplicationTitleSection.propTypes = {
devicesErrored: PropTypes.bool.isRequired,
devicesTotalCount: PropTypes.number,
fetching: PropTypes.bool.isRequired,
lastSeen: PropTypes.string,
linkStats: PropTypes.applicationLinkStats,
Copy link
Contributor

Choose a reason for hiding this comment

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

You can also edit the applicationLink and applicationLinkStats prop-type definitions to match the current types.

Copy link
Contributor

Choose a reason for hiding this comment

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

ApplicationLink does not require network_server_address and api_key as those will be deprecated in v3.11

adriansmares
adriansmares reviewed Dec 28, 2020
View reviewed changes
Copy link
Contributor

@adriansmares adriansmares left a comment

Choose a reason for hiding this comment

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

I've tested it locally and works as expected. Test procedure:

  • Create application
  • Create end device
  • Set application uplink payload formatters to the default JS formatter
  • Send simulated uplink and check uplink contents (contains correct decoded_payload)

@asmulko asmulko force-pushed the issue/3542-remove-linking-references branch from 19b7592 to 7096dd2 Compare December 28, 2020 13:00
@asmulko asmulko merged commit 690574d into v3.11 Dec 29, 2020
@asmulko asmulko deleted the issue/3542-remove-linking-references branch December 29, 2020 09:33
@asmulko asmulko linked an issue Dec 29, 2020 that may be closed by this pull request
Remove linking references from the Console #3542
Closed
@asmulko asmulko mentioned this pull request Jan 10, 2021
Closed
65 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adriansmares adriansmares adriansmares left review comments

@bafonins bafonins bafonins approved these changes

@kschiffer kschiffer Awaiting requested review from kschiffer

@pgalic96 pgalic96 Awaiting requested review from pgalic96

Assignees

@asmulko asmulko

Labels
c/console This is related to the Console ui/web This is related to a web interface
Projects
None yet
Milestone
December 2020
Development

Successfully merging this pull request may close these issues.

Remove linking references from the Console
3 participants
@asmulko @bafonins @adriansmares