feat: Add alias to groupBy related object

[shahzadlone]

Resolves #1578
Resolves #1551
Part-of: #1279

Description

• Add some tests of existing object_id functionality.
• Add implementation for groupBy alias.
• Add tests for groupBy alias ability.
• Fix the previous panic using alias, with more user friendly error.
• Add tests for panic fix.
• Make a const for _id.
• Schema Test to ensure the field we want to alias exists (even though it actually existed from before)

For Reviewers

• Commit by commit review should be clean.
• PR(MAIN): commit needs the most attention.
• Happy to add more tests if you can think of any cases.
• Happy to remove tests if I introduced any duplicates.

Tasks

• I made sure the code is well commented, particularly hard-to-understand areas.
• I made sure the repository-held documentation is changed accordingly.
• I made sure the pull request title adheres to the conventional commit style (the subset used in the project can be found in tools/configs/chglog/config.yml).
• I made sure to discuss its limitations such as threats to validity, vulnerability to mistake and misuse, robustness to invalidation of assumptions, resource requirements, ...

How has this been tested?

Integration Tests

Specify the platform(s) on which this was tested:

• Manjaro/Arch WSL2

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.02 🎉

Comparison is base (be619fd) 73.47% compared to head (ff9c480) 73.50%.

@@             Coverage Diff             @@
##           develop    #1579      +/-   ##
===========================================
+ Coverage    73.47%   73.50%   +0.02%     
===========================================
  Files          187      188       +1     
  Lines        19291    19320      +29     
===========================================
+ Hits         14174    14200      +26     
- Misses        4069     4071       +2     
- Partials      1048     1049       +1     

Flag	Coverage Δ
all-tests	73.50% <100.00%> (+0.02%)	⬆️

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

Impacted Files	Coverage Δ
client/request/select.go	100.00% <100.00%> (ø)
db/collection_update.go	72.91% <100.00%> (+0.13%)	⬆️
planner/mapper/errors.go	100.00% <100.00%> (ø)
planner/mapper/mapper.go	86.63% <100.00%> (+0.32%)	⬆️
planner/type_join.go	74.87% <100.00%> (ø)

... and 1 file with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

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

[fredcarle]

Changes look good Shahzad. I'm just wondering about the author_id being returned in the results without it being requested in the query. Was this behavious previously the same? In my mind that would be a bug.

[AndrewSisley]

Changes look good Shahzad. I'm just wondering about the author_id being returned in the results without it being requested in the query. Was this behavious previously the same? In my mind that would be a bug

I think that is also against the GQL spec, IIRC only (and all) fields requested must be returned.

[shahzadlone]

Changes look good Shahzad. I'm just wondering about the author_id being returned in the results without it being requested in the query. Was this behavious previously the same? In my mind that would be a bug.

I thought it was odd too and against the spec, but I am pretty sure that is the default behavior that already existed.

For example:

query {
	Users(groupBy: [Name]) {
		_group {
			Age
		}
	}
}

Would probably give:

[
	{
		{
			"Name": ...
			"_group": [
				{
					{
						"Age": ...
					}
					...
					{
						"Age": ...
					}
				]
			}
		}
		...
		{
			"Name": ...
			"_group": [
				{
					{
						"Age": ...
					}
					...
					{
						"Age": ...
					}
				]
			}
		}
	}
]

[shahzadlone]

I will add a test case to confirm the behavior.

I think that is also against the GQL spec, IIRC only (and all) fields requested must be returned.

Can create an issue if this wasn't intentional for some thought-out reason.

[fredcarle]

LGTM. I opened an issue so we can come back to fixing the inconsistency with the specs #1582

Thanks for adding the test. Maybe before merging, it would be nice to add a statement in the test that states that this is not wanted behaviour.