feat: Add new new push notification interface `ParseNotification` for managing push notifications #949

mbfakourii · 2023-07-03T06:14:25Z

Pull Request

Report security issues confidentially.
Any contribution is under this license.
Link this pull request to an issue.

Issue

Closes: #936

Approach

In the new method, ParseNotification should be given to ParsePush

ParsePush.instance.initialize(
  firebaseMessaging,
  parseNotification: ParseNotification(onShowNotification: (value) {
    // show notification
  }),
);

Tasks

Add tests
Add changes to documentation (guides, repository pages, code comments)

refactor: Add `CONTRIBUTING.md`

cheng parse_push and parse_notification

parse-github-assistant · 2023-07-03T06:14:27Z

I will reformat the title to use the proper commit message syntax.

parse-github-assistant · 2023-07-03T06:14:30Z

Thanks for opening this pull request!

codecov · 2023-07-03T06:16:33Z

Codecov Report

Patch coverage has no change and project coverage change: +0.27% 🎉

Comparison is base (aefc84e) 39.33% compared to head (b6146ec) 39.60%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #949      +/-   ##
==========================================
+ Coverage   39.33%   39.60%   +0.27%     
==========================================
  Files          60       60              
  Lines        3356     3333      -23     
==========================================
  Hits         1320     1320              
+ Misses       2036     2013      -23

Files Changed	Coverage Δ
packages/flutter/lib/parse_server_sdk_flutter.dart	`20.00% <ø> (ø)`
...utter/lib/src/notification/parse_notification.dart	`0.00% <0.00%> (ø)`
packages/flutter/lib/src/push/parse_push.dart	`0.00% <0.00%> (ø)`

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mtrezza · 2023-07-10T23:42:13Z

@cool2apps Could you test this PR out, if it works for you?

mbfakourii · 2023-07-19T17:36:17Z

@cool2apps Could you test this PR out, if it works for you?

I think it is ready for merge, it is no longer dependent on another library

mtrezza

Could you please change the PR title? It currently seems to describe the change, but it should describe the issue that is being fixed.

mbfakourii · 2023-07-21T06:37:27Z

Could you please change the PR title? It currently seems to describe the change, but it should describe the issue that is being fixed.

I changed it, I hope it's good, I didn't have a better idea for the title

mtrezza · 2023-07-21T22:28:26Z

The PR title must describe the issue, the solution of how it was fixed is an option addition. It's still not clear what this PR fixes. If it'S not "properly" displaying notifications, could we specify what that means? Or is it something else?

mbfakourii · 2023-07-22T07:00:24Z

The PR title must describe the issue, the solution of how it was fixed is an option addition. It's still not clear what this PR fixes. If it'S not "properly" displaying notifications, could we specify what that means? Or is it something else?

I changed it, what do you think?

mtrezza · 2023-07-22T22:58:20Z

Again, this describes the change, not the issue that this PR fixes.

Is this PR fixing a bug, then what does it fix?
Is the PR adding a new or changing an existing feature and not fixing a bug, then what is that feature?

mbfakourii · 2023-07-24T20:57:44Z

Again, this describes the change, not the issue that this PR fixes.

Is this PR fixing a bug, then what does it fix?

Is the PR adding a new or changing an existing feature and not fixing a bug, then what is that feature?

In this PR, we solved the problem of using the notification library overhead by removing it and replacing it with an interface.
I think this PR solves a bug

mtrezza · 2023-07-24T21:20:03Z

If it was a bug we should be able to describe what's broken or not working, but that seems not possible. If it removes a overhead then maybe it's a performance improvement?

How about:

perf: Remove push notification library overhead by replacing it with a push notification interface

Just to confirm again, this PR does not require any changes in a developer's app code, right?

mbfakourii · 2023-07-25T06:38:48Z

If it was a bug we should be able to describe what's broken or not working, but that seems not possible. If it removes a overhead then maybe it's a performance improvement?

How about:

perf: Remove push notification library overhead by replacing it with a push notification interface

Just to confirm again, this PR does not require any changes in a developer's app code, right?

Changes are required on the developer side.
I think this is a breaking changes

but since ParsePush is a new feature and this change is very small, I'm not sure there is a need for a MAJOR version increase.

mtrezza · 2023-07-25T10:47:03Z

We do not distinguish between "small" and "large" breaking changes. Any breaking change usually requires a major version increment, the exception being security fixes which are evaluated on a per-case basis.

but since ParsePush is a new feature

So then this is would be a feat PR that contains a breaking change. A feat is higher in priority than a perf change, but we can add the performance implication in the changelog entry.

Merging a breaking change as non-breaking in flutter is especially bad due to the de-facto standard of using range operators for dependency versions. That means they will auto-update and potentially break apps. We need to be extra careful because of that.

mbfakourii · 2023-07-28T07:31:40Z

We do not distinguish between "small" and "large" breaking changes. Any breaking change usually requires a major version increment, the exception being security fixes which are evaluated on a per-case basis.

but since ParsePush is a new feature

So then this is would be a feat PR that contains a breaking change. A feat is higher in priority than a perf change, but we can add the performance implication in the changelog entry.

Merging a breaking change as non-breaking in flutter is especially bad due to the de-facto standard of using range operators for dependency versions. That means they will auto-update and potentially break apps. We need to be extra careful because of that.

I got it, so it is a breaking change and feat added in title

mtrezza · 2023-07-28T21:30:45Z

I think the PR title could be improved; with "user interface" you are probably referring to the programming interface, not a UI.

How about this change log entry:

BREAKING CHANGES

The push notification library flutter_local_notifications is replaced with the new push notification interface ParseNotification (#949)

Features

Add new new push notification interface ParseNotification for managing push notifications (#949)

Feel free to add any info to the "breaking change" line that may help developers to cope with the braking change.

mbfakourii · 2023-07-30T07:29:32Z

I think the PR title could be improved; with "user interface" you are probably referring to the programming interface, not a UI.

How about this change log entry:

BREAKING CHANGES

The push notification library flutter_local_notifications is replaced with the new push notification interface ParseNotification (#949)

Features

Add new new push notification interface ParseNotification for managing push notifications (#949)

Feel free to add any info to the "breaking change" line that may help developers to cope with the braking change.

I changed it, what do you think?

mtrezza · 2023-07-30T21:13:51Z

We would need a changelog entry as well

mbfakourii · 2023-08-02T10:22:12Z

We would need a changelog entry as well

added

packages/flutter/CHANGELOG.md

mtrezza · 2023-08-06T16:08:49Z

@mbfakourii I've edited the changelog; to better explain the format, when there's a breaking change, there is the BREAKING CHANGES section that can be more verbose and explain what is breaking, how the developer can transition and what to be careful about. Then there is still the Features section that has the shorter entry for the feature change. In other words, a PR always requires a base entry like Bug Fixes or Feature and if it's breaking it requires an additional entry in BREAKING CHANGES.

mtrezza

Looks good

mbfakourii and others added 12 commits May 25, 2023 09:12

Create CONTRIBUTING.md

cde971b

Merge pull request #1 from mbfakourii/mbfakourii-patch-1

4e7ba14

refactor: Add `CONTRIBUTING.md`

just some styling

835a1db

Merge branch 'parse-community:master' into master

2b6997f

Merge branch 'parse-community:master' into master

45928d4

Merge branch 'parse-community:master' into master

f86d134

Merge branch 'parse-community:master' into master

79f9d4b

Merge branch 'parse-community:master' into master

e3e2b15

Merge branch 'parse-community:master' into master

1b96d48

Merge branch 'parse-community:master' into master

2ac7344

Merge branch 'parse-community:master' into master

9c77278

remove flutter local_notification

f2cab17

cheng parse_push and parse_notification

parse-github-assistant bot changed the title ~~fix: remove flutter_local_notifications and creating a new method for ParseNotification~~ fix: Remove flutter_local_notifications and creating a new method for ParseNotification Jul 3, 2023

mbfakourii requested a review from a team July 6, 2023 11:06

Merge branch 'master' into fix_notification

5328012

Merge branch 'master' into fix_notification

7560d36

mtrezza requested changes Jul 20, 2023

View reviewed changes

mbfakourii changed the title ~~fix: Remove flutter_local_notifications and creating a new method for ParseNotification~~ fix: Update ParseNotification input for proper notification display Jul 21, 2023

mbfakourii changed the title ~~fix: Update ParseNotification input for proper notification display~~ fix: Use a new user interface for managing notifications Jul 22, 2023

mbfakourii added the state:breaking label Jul 25, 2023

mbfakourii changed the title ~~fix: Use a new user interface for managing notifications~~ feat: Use a new user interface for managing notifications Jul 28, 2023

mbfakourii changed the title ~~feat: Use a new user interface for managing notifications~~ feat: Add new new push notification interface ParseNotification for managing push notifications Jul 30, 2023

bump version to 6.0.0 and add entrys

b4ee104

mtrezza reviewed Aug 6, 2023

View reviewed changes

packages/flutter/CHANGELOG.md Outdated Show resolved Hide resolved

Update packages/flutter/CHANGELOG.md

93b4fb1

mtrezza reviewed Aug 6, 2023

View reviewed changes

packages/flutter/CHANGELOG.md Show resolved Hide resolved

Update packages/flutter/CHANGELOG.md

a0dc924

mtrezza reviewed Aug 6, 2023

View reviewed changes

packages/flutter/CHANGELOG.md Outdated Show resolved Hide resolved

Update packages/flutter/CHANGELOG.md

8295a1c

mtrezza reviewed Aug 6, 2023

View reviewed changes

packages/flutter/CHANGELOG.md Outdated Show resolved Hide resolved

Update packages/flutter/CHANGELOG.md

b6146ec

mtrezza approved these changes Aug 6, 2023

View reviewed changes

mtrezza merged commit a2fb099 into parse-community:master Aug 6, 2023

mbfakourii deleted the fix_notification branch October 3, 2023 14:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add new new push notification interface `ParseNotification` for managing push notifications #949

feat: Add new new push notification interface `ParseNotification` for managing push notifications #949

mbfakourii commented Jul 3, 2023 •

edited by mtrezza

Loading

parse-github-assistant bot commented Jul 3, 2023

parse-github-assistant bot commented Jul 3, 2023

codecov bot commented Jul 3, 2023 •

edited

Loading

mtrezza commented Jul 10, 2023 •

edited

Loading

mbfakourii commented Jul 19, 2023

mtrezza left a comment

mbfakourii commented Jul 21, 2023 •

edited

Loading

mtrezza commented Jul 21, 2023 •

edited

Loading

mbfakourii commented Jul 22, 2023

mtrezza commented Jul 22, 2023 •

edited

Loading

mbfakourii commented Jul 24, 2023

mtrezza commented Jul 24, 2023 •

edited

Loading

mbfakourii commented Jul 25, 2023 •

edited

Loading

mtrezza commented Jul 25, 2023 •

edited

Loading

mbfakourii commented Jul 28, 2023

mtrezza commented Jul 28, 2023 •

edited

Loading

mbfakourii commented Jul 30, 2023

mtrezza commented Jul 30, 2023

mbfakourii commented Aug 2, 2023

mtrezza commented Aug 6, 2023

mtrezza left a comment

feat: Add new new push notification interface ParseNotification for managing push notifications #949

feat: Add new new push notification interface ParseNotification for managing push notifications #949

Conversation

mbfakourii commented Jul 3, 2023 • edited by mtrezza Loading

Pull Request

Issue

Approach

Tasks

parse-github-assistant bot commented Jul 3, 2023

parse-github-assistant bot commented Jul 3, 2023

Thanks for opening this pull request!

codecov bot commented Jul 3, 2023 • edited Loading

Codecov Report

mtrezza commented Jul 10, 2023 • edited Loading

mbfakourii commented Jul 19, 2023

mtrezza left a comment

Choose a reason for hiding this comment

mbfakourii commented Jul 21, 2023 • edited Loading

mtrezza commented Jul 21, 2023 • edited Loading

mbfakourii commented Jul 22, 2023

mtrezza commented Jul 22, 2023 • edited Loading

mbfakourii commented Jul 24, 2023

mtrezza commented Jul 24, 2023 • edited Loading

mbfakourii commented Jul 25, 2023 • edited Loading

mtrezza commented Jul 25, 2023 • edited Loading

mbfakourii commented Jul 28, 2023

mtrezza commented Jul 28, 2023 • edited Loading

mbfakourii commented Jul 30, 2023

mtrezza commented Jul 30, 2023

mbfakourii commented Aug 2, 2023

mtrezza commented Aug 6, 2023

mtrezza left a comment

Choose a reason for hiding this comment

feat: Add new new push notification interface `ParseNotification` for managing push notifications #949

feat: Add new new push notification interface `ParseNotification` for managing push notifications #949

mbfakourii commented Jul 3, 2023 •

edited by mtrezza

Loading

codecov bot commented Jul 3, 2023 •

edited

Loading

mtrezza commented Jul 10, 2023 •

edited

Loading

mbfakourii commented Jul 21, 2023 •

edited

Loading

mtrezza commented Jul 21, 2023 •

edited

Loading

mtrezza commented Jul 22, 2023 •

edited

Loading

mtrezza commented Jul 24, 2023 •

edited

Loading

mbfakourii commented Jul 25, 2023 •

edited

Loading

mtrezza commented Jul 25, 2023 •

edited

Loading

mtrezza commented Jul 28, 2023 •

edited

Loading