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

Add notifications for linked users #142

Merged
merged 1 commit into from
Sep 17, 2024
Merged

Add notifications for linked users #142

merged 1 commit into from
Sep 17, 2024

Conversation

Hunterness
Copy link
Contributor

@Hunterness Hunterness commented May 7, 2024 •
edited
Loading

Notifications introduced in https://github.com/neo-technology/neo4j/pull/25227 (merged, but behind feature flag). Feature flag removed in https://github.com/neo-technology/neo4j/pull/26923.

Hunterness
Hunterness commented May 7, 2024
View reviewed changes
modules/ROOT/pages/notifications/all-notifications.adoc
[cols="<1s,<4"]
|===
|Code
m|Neo.ClientNotification.Security.AuthProviderNotDefined
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This notification was covered by CIP-145.

modules/ROOT/pages/notifications/all-notifications.adoc
[cols="<1s,<4"]
|===
|Code
m|Neo.ClientNotification.Security.ExternalAuthNotEnabled
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This notification was an error in CIP-145 but was decided to be just a warning during implementation (to not lock out current external only users from being able to switch by first adding the new external only users to system before disabling their ways to log in)

modules/ROOT/pages/notifications/all-notifications.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/notifications/all-notifications.adoc Show resolved Hide resolved
@renetapopova renetapopova self-requested a review May 9, 2024 09:19
renetapopova
renetapopova approved these changes May 13, 2024
View reviewed changes
Copy link
Collaborator

@renetapopova renetapopova left a comment

Choose a reason for hiding this comment

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

Some minor editorial suggestions. Otherwise, it looks good from my side.

modules/ROOT/pages/notifications/all-notifications.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/notifications/all-notifications.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/notifications/all-notifications.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/notifications/all-notifications.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/notifications/all-notifications.adoc Outdated Show resolved Hide resolved
@Hunterness Hunterness force-pushed the dev-linked-users-notifications branch from 52242a7 to 2d5382c Compare June 12, 2024 07:07
@Hunterness
Copy link
Contributor Author

@renetapopova The PR introducing these notifications has been merged. However the feature is still behind a feature flag and not fully done, should we merge this already now or should it wait for the feature to be GA?

@renetapopova
Copy link
Collaborator

renetapopova commented Jun 12, 2024 •
edited
Loading

Are these notifications visible to the user?

@Hunterness
Copy link
Contributor Author

Hunterness commented Jun 12, 2024 •
edited
Loading

Are these notifications visible to the user?

If you turn on the feature flag (an internal setting) and try out the linked users feature and trigger them, yes. If you don't turn on the feature flag then no.

@renetapopova
Copy link
Collaborator

Are these notifications visible to the user?

If you turn on the feature flag (an internal setting) and try out the linked users feature and trigger them, yes. If you don't turn on the feature flag then no.

We don't surface internal settings, so I guess we don't need to merge it for now.

@renetapopova renetapopova removed the 5.21 label Jun 13, 2024
@NataliaIvakina NataliaIvakina self-assigned this Jun 13, 2024
@renetapopova
Copy link
Collaborator

Related to neo4j/docs-operations#1745

@Hunterness Hunterness added 5.24 and removed 5.25 labels Aug 29, 2024
@Hunterness
Copy link
Contributor Author

Updated version tag to match the related docs PR

@Hunterness Hunterness force-pushed the dev-linked-users-notifications branch from 599e7d7 to e842ee7 Compare September 9, 2024 15:11
@renetapopova renetapopova force-pushed the dev-linked-users-notifications branch from e842ee7 to d4181a6 Compare September 17, 2024 14:48
@neo-technology-commit-status-publisher
Copy link
Collaborator

Thanks for the documentation updates.

The preview documentation has now been torn down - reopening this PR will republish it.

@renetapopova renetapopova merged commit e958617 into neo4j:dev Sep 17, 2024
5 checks passed
NataliaIvakina pushed a commit to NataliaIvakina/docs-status-codes that referenced this pull request Sep 26, 2024
@Hunterness @NataliaIvakina
Add notifications for linked users (neo4j#142)
3ec64fe 
Notifications introduced in
neo-technology/neo4j#25227 (merged, but behind
feature flag). Feature flag removed in
neo-technology/neo4j#26923.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@renetapopova renetapopova renetapopova approved these changes

Assignees

@NataliaIvakina NataliaIvakina

Labels
5.24
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Hunterness @renetapopova @neo-technology-commit-status-publisher @NataliaIvakina