Fix the first contributor filter #891

orhun · 2024-09-27T15:24:32Z

Is there an existing issue for this?

I have searched the existing issues

Description of the bug

In the following scenario:

User foo contributes for the first time.
New release is created and changelog is generated.
User foo contributes after the release.
New release is created and changelog is generated BUT user foo is removed from the first contributors section from the previous release.

Might be related to #825

Steps To Reproduce

See above.

Expected behavior

This happens due to not checking the dates of the new commits while updating the release metadata.

git-cliff/git-cliff-core/src/remote/mod.rs

Lines 337 to 347 in 8d10edb

    
           // mark contributors as first-time 
        
           self.$remote.contributors = contributors 
        
           	.into_iter() 
        
           	.map(|mut v| { 
        
           		v.is_first_time = !commits 
        
           			.iter() 
        
           			.map(|v| v.username()) 
        
           			.any(|login| login == v.username); 
        
           		v 
        
           	}) 
        
           	.collect();

Here, we check all the commits, but you see, in the new release user foo already contributed, so his contribution is not marked as first time anymore.

To fix it, we should simply:

diff --git a/git-cliff-core/src/remote/mod.rs b/git-cliff-core/src/remote/mod.rs
index 77d14e7..32e61a9 100644
--- a/git-cliff-core/src/remote/mod.rs
+++ b/git-cliff-core/src/remote/mod.rs
@@ -83,6 +83,8 @@ pub trait RemoteCommit: DynClone {
 	fn id(&self) -> String;
 	/// Commit author.
 	fn username(&self) -> Option<String>;
+	/// Timestamp.
+	fn timestamp(&self) -> Option<i64>;
 }
 
 dyn_clone::clone_trait_object!(RemoteCommit);
@@ -340,6 +342,9 @@ macro_rules! update_release_metadata {
 					.map(|mut v| {
 						v.is_first_time = !commits
 							.iter()
+							.filter(|commit| {
+								commit.timestamp() < Some(self.timestamp)
+							})
 							.map(|v| v.username())
 							.any(|login| login == v.username);
 						v

Apply the patch above and you will see that we are missing the implementation for GitHub, GitLab, etc. We need to check each remote and add the appropriate field for serializing the timestamp from their API response.

Screenshots / Logs

nA

Software information

nA

Additional context

No response

jaiakash · 2024-09-30T21:34:32Z

Hi @orhun can i work on this?

orhun · 2024-10-01T08:54:05Z

please do! @jaiakash

orhun · 2024-10-06T10:41:58Z

any progress/ETA about this @jaiakash? I think this is affecting more users than I was expecting #906

mcwarman · 2024-10-07T07:26:34Z

I tested the suggested fix on mcwarman-playground/git-cliff-readme-example.

And something isn't quite correct in the logic, it repots orhun as new contributor again:

## What's Changed
* feat(config): support multiple file formats by @orhun <[email protected]>
* feat(cache): use cache while fetching pages by @orhun <[email protected]>
* feat(ci): add pipeline config by @mcwarman <[email protected]> in #1

### New Contributors
* @mcwarman <[email protected]> made their first contribution in [#1](https://bitbucket.org/mcwarman-playground/git-cliff-readme-example/pull-requests/1)
* @orhun <[email protected]> made their first contribution

## What's Changed in v1.0.1
* refactor(parser): expose string functions by @orhun <[email protected]>
* chore(release): add release script by @orhun <[email protected]>

## What's Changed in v1.0.0
* Initial commit by @orhun <[email protected]>
* docs(project): add README.md by @orhun <[email protected]>
* feat(parser): add ability to parse arrays by @orhun <[email protected]>
* fix(args): rename help argument due to conflict by @orhun <[email protected]>
* docs(example)!: add tested usage example by @orhun <[email protected]>

### New Contributors
* @orhun <[email protected]> made their first contribution

<!-- generated by -cliff -->

diff --git a/git-cliff-core/src/remote/bitbucket.rs b/git-cliff-core/src/remote/bitbucket.rs
index a3e3603..26ef85a 100644
--- a/git-cliff-core/src/remote/bitbucket.rs
+++ b/git-cliff-core/src/remote/bitbucket.rs
@@ -1,5 +1,6 @@
 use crate::config::Remote;
 use crate::error::*;
+use chrono::DateTime;
 use reqwest_middleware::ClientWithMiddleware;
 use serde::{
        Deserialize,
@@ -33,6 +34,8 @@ pub(crate) const BITBUCKET_MAX_PAGE_PRS: usize = 50;
 pub struct BitbucketCommit {
        /// SHA.
        pub hash:   String,
+       /// SHA.
+       pub date:   String,
        /// Author of the commit.
        pub author: Option<BitbucketCommitAuthor>,
 }
@@ -45,6 +48,15 @@ impl RemoteCommit for BitbucketCommit {
        fn username(&self) -> Option<String> {
                self.author.clone().and_then(|v| v.login)
        }
+
+       fn timestamp(&self) -> Option<i64> {
+               debug!("{} {}", self.hash.clone(), self.date.clone());
+               Some(
+                       DateTime::parse_from_rfc3339(&self.date)
+                               .unwrap()
+                               .timestamp(),
+               )
+       }
 }

diff --git a/git-cliff-core/src/remote/mod.rs b/git-cliff-core/src/remote/mod.rs
index a79137b..d4a9681 100644
--- a/git-cliff-core/src/remote/mod.rs
+++ b/git-cliff-core/src/remote/mod.rs
@@ -82,6 +82,8 @@ pub trait RemoteCommit: DynClone {
        fn id(&self) -> String;
        /// Commit author.
        fn username(&self) -> Option<String>;
+       /// Timestamp.
+       fn timestamp(&self) -> Option<i64>;
 }

 dyn_clone::clone_trait_object!(RemoteCommit);
@@ -342,6 +344,9 @@ macro_rules! update_release_metadata {
                                        .map(|mut v| {
                                                v.is_first_time = !commits
                                                        .iter()
+                                                       .filter(|commit| {
+                                                               commit.timestamp() < Some(self.timestamp)
+                                                       })
                                                        .map(|v| v.username())
                                                        .any(|login| login == v.username);
                                                v

orhun · 2024-10-08T06:50:50Z

I guess we still need to check the past releases... the logic here boggles my mind a bit.

We are filtering commits by timestamp to exclude those that occurred before the current release's timestamp.
However, this doesn't fully account for the fact that is_first_time is being evaluated globally, not per release.

🤔

mcwarman · 2024-10-08T10:47:05Z

~~I wasn't sure if it was some how not filtering the username correctly, and mcwarman first time contribution was marking both contributors as true.~~

~~The only reason i thought it might be this is because v1.0.1 doesn't have a new contributors section in again.~~

I now understand this a little bit more after some debugging.

mcwarman · 2024-10-08T22:25:42Z

Not sure if something like this works:

.filter(|commit| {
	//unreleased
	self.timestamp == 0 ||
		commit.timestamp() < Some(self.timestamp)
})

orhun · 2024-10-08T23:32:37Z

Yes, I think something like that might work. Are you getting the correct results with it?

mcwarman · 2024-10-09T07:44:26Z

For the Bitbucket it was, but possibly more by the luck

The approach isn't flawless, because I realised self.timestamp is the tag timestamp, and that differs to the commit related to that tag timestamp so the filter doesn't do the 100% the correct thing but gets you close.

orhun · 2024-10-14T16:04:16Z

Hmm I see.. Either way, can you create a PR with your changes? I think anything that gets us to a more accurate result is welcome :)

orhun · 2024-10-14T16:05:20Z

Also I just updated the issue labels. This turned out to be more difficult than I expected. I'm still not sure about the logic above.

mcwarman · 2024-10-19T09:17:46Z

I think this works:

Store the remote commit timestamp of the current release (while iterating)
Use that timestamp to perform the filter

diff --git a/git-cliff-core/src/remote/mod.rs b/git-cliff-core/src/remote/mod.rs
index a79137b..d83fe28 100644
--- a/git-cliff-core/src/remote/mod.rs
+++ b/git-cliff-core/src/remote/mod.rs
@@ -82,6 +82,8 @@ pub trait RemoteCommit: DynClone {
 	fn id(&self) -> String;
 	/// Commit author.
 	fn username(&self) -> Option<String>;
+	/// Timestamp.
+	fn timestamp(&self) -> Option<i64>;
 }
 
 dyn_clone::clone_trait_object!(RemoteCommit);
@@ -299,6 +301,7 @@ macro_rules! update_release_metadata {
 				pull_requests: Vec<Box<dyn RemotePullRequest>>,
 			) -> Result<()> {
 				let mut contributors: Vec<RemoteContributor> = Vec::new();
+				let mut release_commit_timestamp: Option<i64> = None;
 				// retain the commits that are not a part of this release for later
 				// on checking the first contributors.
 				commits.retain(|v| {
@@ -331,6 +334,11 @@ macro_rules! update_release_metadata {
 							});
 						}
 						commit.remote = Some(commit.$remote.clone());
+						// if remote commit is the release commit store timestamp for
+						// use in calculation of first time
+						if Some(v.id().clone()) == self.commit_id {
+							release_commit_timestamp = v.timestamp().clone();
+						}
 						false
 					} else {
 						true
@@ -342,6 +350,12 @@ macro_rules! update_release_metadata {
 					.map(|mut v| {
 						v.is_first_time = !commits
 							.iter()
+							.filter(|commit| {
+								// if current release is unreleased no need to filter
+								// commits or filter commits that are from
+								// newer releases
+								self.timestamp == 0 ||
+									commit.timestamp() < release_commit_timestamp
+							})
 							.map(|v| v.username())
 							.any(|login| login == v.username);
 						v

Although to make it work bitbucket I had to override the endpoint because of #906

## What's Changed
* feat(config): support multiple file formats by @orhun <[email protected]>
* feat(cache): use cache while fetching pages by @orhun <[email protected]>
* feat(ci): add pipeline config by @mcwarman <[email protected]> in #1

### New Contributors
* @mcwarman <[email protected]> made their first contribution in [#1](https://bitbucket.org/mcwarman-playground/git-cliff-readme-example/pull-requests/1)

## What's Changed in v1.0.1
* refactor(parser): expose string functions by @orhun <[email protected]>
* chore(release): add release script by @orhun <[email protected]>

## What's Changed in v1.0.0
* Initial commit by @orhun <[email protected]>
* docs(project): add README.md by @orhun <[email protected]>
* feat(parser): add ability to parse arrays by @orhun <[email protected]>
* fix(args): rename help argument due to conflict by @orhun <[email protected]>
* docs(example)!: add tested usage example by @orhun <[email protected]>

### New Contributors
* @orhun <[email protected]> made their first contribution

<!-- generated by -cliff -->

orhun added bug Something isn't working good first issue Good for newcomers labels Sep 27, 2024

orhun assigned jaiakash Oct 1, 2024

orhun mentioned this issue Oct 6, 2024

Bitbucket is_first_time can report incorrectly #906

Open

1 task

orhun unassigned jaiakash Oct 6, 2024

orhun added the integration Related to remote integration label Oct 6, 2024

orhun added help wanted Extra attention is needed and removed good first issue Good for newcomers labels Oct 14, 2024

mcwarman mentioned this issue Oct 19, 2024

fix(remote): preserve first time contributors #925

Merged

12 tasks

orhun closed this as completed in #925 Oct 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the first contributor filter #891

Fix the first contributor filter #891

orhun commented Sep 27, 2024

jaiakash commented Sep 30, 2024

orhun commented Oct 1, 2024

orhun commented Oct 6, 2024

mcwarman commented Oct 7, 2024

orhun commented Oct 8, 2024

mcwarman commented Oct 8, 2024 •

edited

Loading

mcwarman commented Oct 8, 2024

orhun commented Oct 8, 2024

mcwarman commented Oct 9, 2024 •

edited

Loading

orhun commented Oct 14, 2024

orhun commented Oct 14, 2024

mcwarman commented Oct 19, 2024 •

edited

Loading

Fix the first contributor filter #891

Fix the first contributor filter #891

Comments

orhun commented Sep 27, 2024

Is there an existing issue for this?

Description of the bug

Steps To Reproduce

Expected behavior

Screenshots / Logs

Software information

Additional context

jaiakash commented Sep 30, 2024

orhun commented Oct 1, 2024

orhun commented Oct 6, 2024

mcwarman commented Oct 7, 2024

orhun commented Oct 8, 2024

mcwarman commented Oct 8, 2024 • edited Loading

mcwarman commented Oct 8, 2024

orhun commented Oct 8, 2024

mcwarman commented Oct 9, 2024 • edited Loading

orhun commented Oct 14, 2024

orhun commented Oct 14, 2024

mcwarman commented Oct 19, 2024 • edited Loading

mcwarman commented Oct 8, 2024 •

edited

Loading

mcwarman commented Oct 9, 2024 •

edited

Loading

mcwarman commented Oct 19, 2024 •

edited

Loading