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

Separate user and admin notifications #1027

Merged
merged 1 commit into from
Jun 3, 2021
Merged

Conversation

paskal
Copy link
Collaborator

@paskal paskal commented May 25, 2021

The current state is a mess of user and admin notifications, which will become worse after implementing the new user notification methods like a telegram.

This change makes things simpler for the remark42 users.

Can be merged after #1026 will be merged.

@paskal paskal requested a review from umputun May 25, 2021 17:14
@paskal paskal changed the title separate user and admin notifications Separate user and admin notifications May 25, 2021
@paskal paskal force-pushed the paskal/clarify_notifications branch from 140a2a2 to 0077e3e Compare May 25, 2021 17:17
@paskal paskal added the backend label May 25, 2021
@paskal paskal force-pushed the paskal/clarify_notifications branch from 0077e3e to 1e0d575 Compare May 25, 2021 20:25
@codecov
Copy link

codecov bot commented May 25, 2021

Codecov Report

Merging #1027 (0321c4f) into master (97934e2) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1027   +/-   ##
=======================================
  Coverage   43.97%   43.97%           
=======================================
  Files         127      127           
  Lines        2906     2906           
  Branches      658      658           
=======================================
  Hits         1278     1278           
  Misses       1616     1616           
  Partials       12       12           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 97934e2...0321c4f. Read the comment docs.

@paskal paskal marked this pull request as ready for review May 25, 2021 20:28
@coveralls
Copy link

Pull Request Test Coverage Report for Build 876262489

  • 46 of 71 (64.79%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.01%) to 83.673%

Changes Missing Coverage Covered Lines Changed/Added Lines %
backend/app/cmd/server.go 46 71 64.79%
Totals Coverage Status
Change from base Build 876147954: 0.01%
Covered Lines: 5463
Relevant Lines: 6529

💛 - Coveralls

@umputun
Copy link
Owner

umputun commented May 27, 2021

I found the naming to be extremely confusing. As I read UserTypes my brain immediately translates it to "types of users". Not sure what will be the better name here

The current state is a mess of user and admin
notifications, which will become worse after
implementing the new user notification methods
like a telegram.

This change makes things simpler
for the remark42 users.
@paskal paskal force-pushed the paskal/clarify_notifications branch from 1e0d575 to 0321c4f Compare May 31, 2021 18:33
@umputun umputun merged commit c0b392a into master Jun 3, 2021
@umputun umputun deleted the paskal/clarify_notifications branch June 3, 2021 05:30
@paskal paskal added this to the v1.9 milestone Jan 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants