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

Integrate "What's New" Notifications/Updates #5665

Merged
merged 13 commits into from
Aug 24, 2021
Merged

Integrate "What's New" Notifications/Updates #5665

merged 13 commits into from
Aug 24, 2021

Conversation

philippotto
Copy link
Member

@philippotto philippotto commented Aug 11, 2021

URL of deployed dev instance (used for testing):

Steps to test:

  • open dev instance (isDemo is enabled currently)
  • click on "What's New" to see a modal with wK news
  • publish some new news on olvy (hit me up for credentials)
  • reload wk and expect a red dot on the "What's New" button
  • open the modal again --> dot should disappear
  • reload page
  • dot should be gone

Issues:


@philippotto philippotto self-assigned this Aug 11, 2021
@philippotto
Copy link
Member Author

I integrated the feedback I got and now the lastOpenedReleasesTimestamp is stored in the database. Also, the notification indicator is integrated into the avatar. It looks like this:

image
image

@fm3 Please have a look at the back-end code I added. I'm not sure whether I adhered to the existing practices (e.g., the parameter ordering was not very clear to me).

@philippotto philippotto requested a review from fm3 August 18, 2021 14:22
@philippotto
Copy link
Member Author

@fm3 Also maybe can you help with writing a migration which adds the lastOpenedReleasesTimestamp column (could default to lastActivity or to the execution time of the migration if that's necessary)? Happy to have a call about it 🤙

@fm3
Copy link
Member

fm3 commented Aug 23, 2021

@philippotto I pushed a version that moves this timestamp to the multiUser’s novelUserExperienceInfos. Note that I also changed the code slightly to allow for this field to be missing. This way users that never clicked the button will see all that happened since their registration (which is similar to defaulting to the time of the evolution, as we will start publishing entries only then). This way we don’t need a migration or any other back-end changes. Do you agree that this covers the use cases?

conf/application.conf Outdated Show resolved Hide resolved
frontend/javascripts/navbar.js Show resolved Hide resolved
@philippotto
Copy link
Member Author

@philippotto I pushed a version that moves this timestamp to the multiUser’s novelUserExperienceInfos. Note that I also changed the code slightly to allow for this field to be missing. This way users that never clicked the button will see all that happened since their registration (which is similar to defaulting to the time of the evolution, as we will start publishing entries only then). This way we don’t need a migration or any other back-end changes. Do you agree that this covers the use cases?

Thank you for doing this! Somehow I missed that this will be a front-end-only code change then 🤦 But glad to see that it turned out that way 👍

frontend/javascripts/navbar.js Outdated Show resolved Hide resolved
@philippotto philippotto merged commit 96d023a into master Aug 24, 2021
@philippotto philippotto deleted the olvy branch August 24, 2021 11:48
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.

Integrate "What's New" UI
3 participants