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

[#3168] - edit datapoints #3397

Merged
merged 7 commits into from
Feb 4, 2020
Merged

[#3168] - edit datapoints #3397

merged 7 commits into from
Feb 4, 2020

Conversation

marvinkome
Copy link
Contributor

The solution

Add page/view to be able to edit data points in a device.

Screenshots (if appropriate)

image

Reviewer Checklist

  • Added an explanation about the work done
  • Connected the PR and the issue on Zenhub
  • Added a test plan to the issue
  • Updated the copyright header (when relevant)
  • Formatted the code
  • Added a documentation (if relevant)
  • Added some unit tests (if relevant)

@marvinkome marvinkome requested a review from muloem January 21, 2020 08:48
const { devices, datapointAssignments } = this.context.data;

const deviceData = devices.find(device => device.id === selectedDeviceId);
const selectedDatapoint = datapointAssignments.find(
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const selectedDatapoint = datapointAssignments.find(
const selectedDatapointAssignment = datapointAssignments.find(

</span>
<span className="divider">.</span>
<a href="#" onClick={() => this.props.changeTab('EDIT_DATAPOINTS', deviceData.id)}>
Edit
Copy link
Member

Choose a reason for hiding this comment

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

It feels a little strange that the Edit button is active even when there are no datapoints assigned. This should probably only be activated when there are actually datapoints that have been assigned to a device.

Copy link
Member

Choose a reason for hiding this comment

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

Also should be a translated string.

@@ -731,5 +724,31 @@ FLOW.AssignmentEditView = FLOW.ReactComponentView.extend(

this.renderReactSide();
},

removeDatapointsFromAssignments(datapoints, deviceIdInString) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
removeDatapointsFromAssignments(datapoints, deviceIdInString) {
removeDatapointsFromAssignments(datapointIds, deviceIdInString) {

...dpAssignment,
datapoints: dps,
};
});
Copy link
Member

Choose a reason for hiding this comment

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

This section feels a bit confusing the way its constructed. The focus is on the single datapoint assignment that we have to update to reflect that its list of datapointIds has changed. I dont think its necessary to go filtering through all the other datapoint assignments since all we do is just return as long as they dont match the one we want.

@marvinkome marvinkome requested a review from muloem February 4, 2020 12:21
@marvinkome marvinkome merged commit 137a29b into develop Feb 4, 2020
@marvinkome marvinkome deleted the issue-3168/edit-datapoints branch February 4, 2020 12:46
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.

2 participants