Skip to content

Conversation

@heitortanoue
Copy link
Contributor

@heitortanoue heitortanoue commented Sep 4, 2023

Proposed changes (including videos or screenshots)

Added statistic push, which is a number (int) with three bits representing booleans, following the structure:

1 1 1
| | |
| | +- push enabled = 0b1 = 1
| +--- push gateway enabled = 0b10 = 2
+----- push gateway changed = 0b100 = 4

Issue(s)

Steps to test or reproduce

Further comments

TC-875

@changeset-bot
Copy link

changeset-bot bot commented Sep 4, 2023

🦋 Changeset detected

Latest commit: 2677e27

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages
Name Type
@rocket.chat/meteor Minor
@rocket.chat/core-typings Minor
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/gazzodown Major
@rocket.chat/livechat Patch
@rocket.chat/model-typings Patch
@rocket.chat/rest-typings Minor
@rocket.chat/ui-contexts Major
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/stream-hub-service Patch
@rocket.chat/api-client Patch
@rocket.chat/license Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/fuselage-ui-kit Major
@rocket.chat/models Patch
@rocket.chat/ddp-client Patch
@rocket.chat/ui-client Major
@rocket.chat/ui-video-conf Major
@rocket.chat/uikit-playground Patch
@rocket.chat/web-ui-registration Major
@rocket.chat/instance-status Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codecov
Copy link

codecov bot commented Sep 4, 2023

Codecov Report

Merging #30269 (2677e27) into develop (83c7708) will decrease coverage by 0.17%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #30269      +/-   ##
===========================================
- Coverage    50.92%   50.76%   -0.17%     
===========================================
  Files          809      808       -1     
  Lines        15005    15031      +26     
  Branches      2723     2733      +10     
===========================================
- Hits          7642     7631      -11     
- Misses        6954     6978      +24     
- Partials       409      422      +13     
Flag Coverage Δ
e2e 48.43% <ø> (-0.13%) ⬇️
unit 63.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@matheusbsilva137 matheusbsilva137 added this to the 6.5.0 milestone Sep 11, 2023
@matheusbsilva137 matheusbsilva137 marked this pull request as ready for review September 12, 2023 21:08
@matheusbsilva137 matheusbsilva137 requested review from a team as code owners September 12, 2023 21:08
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Sep 13, 2023
Copy link
Member

@sampaiodiego sampaiodiego left a comment

Choose a reason for hiding this comment

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

regarding the reasoning behind the need to have this information added to the statistics, I do think it makes more sense to track Push_enable_gateway than if the gateway was changed or not.. because usually people will either use our apps with our gateway or change do not use our gateway with their own mobile apps.

and since we've been adding too much information to the statistics recently, I'd like to encourage some thinking on how to improve it.. for example, if we're tracking many different push related settings, maybe we can use a binary value? like:

we want to track 3 different settings, each one of them receives receives a bit:

1 1 1
| | |
| | +- push enabled = 0b1 = 
| +--- push gateway enabled = 0b10 = 2
+----- push gateway changed = 0b100 = 4

this way we could have a single value sent to the statistics using bitwise operators, like this:

const pushEnabled = settings.get('Push_enable') ? 1 : 0;
const pushGatewayEnabled = settings.get('Push_enable_gateway') ? 2 : 0;
const gatewayChanged = hasGatewayChanged() ? 4 : 0;

statistics.push = pushEnabled | pushGatewayEnabled | gatewayChanged;

just an idea I think we should consider =)

@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels Sep 20, 2023
@dionisio-bot dionisio-bot bot removed the stat: ready to merge PR tested and approved waiting for merge label Sep 20, 2023
@matheusbsilva137 matheusbsilva137 added the stat: ready to merge PR tested and approved waiting for merge label Oct 3, 2023
@kodiakhq kodiakhq bot merged commit c0ef13a into develop Oct 3, 2023
@kodiakhq kodiakhq bot deleted the feat/push-stats branch October 3, 2023 15:44
@scuciatto scuciatto modified the milestones: 6.5.0, 6.6.0 Oct 4, 2023
gabriellsh added a commit that referenced this pull request Oct 6, 2023
…t into improve/ui-caching

* 'improve/ui-caching' of github.com:RocketChat/Rocket.Chat: (40 commits)
  Update .changeset/wicked-humans-hang.md
  feat: Save visitor's activity on agent's interaction (#30222)
  chore: update mentions upsell modal content (#30503)
  fix: font-disabled color (#30569)
  chore: anchor text-decoration rules (#30485)
  chore: `AccessibilityPage` hide Enterprise tag when is EE (#30486)
  chore: Replace `Field.[SUBCOMPONENT]` in favor of named imports (#30501)
  fix: Check for room scoped permissions for creating discussions (#30497)
  ci: fix Docker image build for production (#30562)
  chore: Fix triggers flaky tests (#30530)
  chore: get translations from CDN (#30331)
  feat: new `licenses.info` endpoint (#30473)
  ci: run tests from forks (#30556)
  regression: unmarked dangling promise on license validation (#30557)
  regression: fix initializing startup order (#30555)
  feat: push notification statistics (#30269)
  chore: do not focus messagebox on mobile devices (#30553)
  chore: add the new endpoints to sync with cloud (#30546)
  fix: in forward search field, user cannot be found by name (Full Name) (#29663)
  chore: set license public key v3 with v2 (#30548)
  ...
debdutdeb pushed a commit that referenced this pull request Oct 26, 2023
@scuciatto scuciatto modified the milestones: 6.6.0, 6.5.0 Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

squad: team-collab stat: ready to merge PR tested and approved waiting for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants