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

NIFI-12972 - Only show selected relationships in read-only connection details #8582

Merged
merged 2 commits into from Apr 3, 2024

Conversation

pvillard31
Copy link
Contributor

Summary

NIFI-12972 - Only show selected relationships in read-only connection details

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

Pull Request Tracking

  • Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
  • Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

  • Pull Request based on current revision of the main branch
  • Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

  • Build completed using mvn clean install -P contrib-check
    • JDK 21

Licensing

  • New dependencies are compatible with the Apache License 2.0 according to the License Policy
  • New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

  • Documentation formatting appears as expected in rendered files

Copy link
Contributor

@mcgilman mcgilman left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @pvillard31! I've left a few notes below. There is a corner case that we should account for.

Comment on lines 502 to 504
$.each(selectedRelationships, function (i, name) {
createRelationshipOption(name);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to account for dynamic Relationships that are selected within this Connection but have since been deleted from the source Processor. Previously we would render all Relationships here and in the block below we would identify any selected relationships that are no longer available and assign them the undefined class. Since now we're only rendering the selected relationships we'll probably want to handle that here and remove the block below.

Note: There is admittedly a bug in the existing implementation because createRelationshipOption is not actually returning the new option. It will need to be updated like:

    var createRelationshipOption = function (name) {
        return $('<div class="available-relationship-container"></div>').append(
            $('<div class="relationship-name"></div>').text(name)).appendTo('#read-only-relationship-names');
    };

You should see the Relationship that was removed is rendered in a lighter color and in italic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Matt for catching this. I just pushed a new commit with the suggested changes.

Screenshot 2024-04-01 at 09 50 11
Screenshot 2024-04-01 at 09 50 20

Copy link
Contributor

@mcgilman mcgilman left a comment

Choose a reason for hiding this comment

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

Thanks for the updates @pvillard31! +1

@mcgilman mcgilman merged commit 644d086 into apache:main Apr 3, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants