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

[WEB-970] Part 1: Add New Property sendMetaCapiEvents #1795

Merged
merged 9 commits into from Feb 24, 2023
Merged

Conversation

scottkicks
Copy link
Contributor

@scottkicks scottkicks commented Feb 21, 2023

πŸ“² What

Add new property to help determine if we need to send CAPI events to the backend

  • sendMetaCapiEvents

πŸ€” Why

This new property will help tell us if we should send CAPI events

πŸ›  How

Added this property to the Project fragment.

...
sendMetaCapiEvents
...

βœ… Acceptance criteria

  • Project page and Dashboard loads and no missing data. User can pledge/back/edit pledge/view comments/view updates, etc. No new functionality added.

@scottkicks scottkicks self-assigned this Feb 21, 2023
@scottkicks scottkicks added this to the release-5.7.0 milestone Feb 21, 2023
@scottkicks scottkicks changed the title [WEB-970] Add New Property sendMetaCapiEvents [WEB-970] Part 1: Add New Property sendMetaCapiEvents Feb 21, 2023
@scottkicks
Copy link
Contributor Author

scottkicks commented Feb 22, 2023

@msadoon For some reason I'm getting this error locally in the auto generated GraphAPI file on build. Its keeping me from testing the currently failing file. I'm wondering if you see the same on your end.
I went through the usual clear cache, derived data, reset packages, etc.. but I think something isn't actually clearing on my end πŸ€”
Screen Shot 2023-02-22 at 12 24 19 PM

@msadoon
Copy link
Contributor

msadoon commented Feb 22, 2023

Weird, I pulled down your branch and it just built right away. Not sure...

@sclampet
Copy link

Weird, I pulled down your branch and it just built right away. Not sure...

Did KsApi test run?

@msadoon
Copy link
Contributor

msadoon commented Feb 22, 2023

Weird, I pulled down your branch and it just built right away. Not sure...

Did KsApi test run?

Yea so it took me bit to find the error, related to some test templates we were using, the last commit should have the fix. :)

@codecov
Copy link

codecov bot commented Feb 22, 2023

Codecov Report

Merging #1795 (578b5b3) into main (dc3831b) will decrease coverage by 0.03%.
The diff coverage is 25.00%.

@@            Coverage Diff             @@
##             main    #1795      +/-   ##
==========================================
- Coverage   85.47%   85.45%   -0.03%     
==========================================
  Files        1282     1282              
  Lines      117246   117291      +45     
  Branches    31037    31042       +5     
==========================================
+ Hits       100213   100226      +13     
- Misses      15958    15990      +32     
  Partials     1075     1075              
Impacted Files Coverage Ξ”
KsApi/models/lenses/ProjectLenses.swift 16.83% <13.55%> (-0.59%) ⬇️
KsApi/models/Project.swift 80.32% <100.00%> (+0.16%) ⬆️
...raphql/adapters/Backing+BackingFragmentTests.swift 98.58% <100.00%> (+<0.01%) ⬆️
.../adapters/Project+FetchProjectQueryDataTests.swift 96.40% <100.00%> (+0.02%) ⬆️
...els/graphql/adapters/Project+ProjectFragment.swift 91.03% <100.00%> (+0.04%) ⬆️
...raphql/adapters/Project+ProjectFragmentTests.swift 97.82% <100.00%> (+0.01%) ⬆️
...ons/templates/query/FetchAddOnsQueryTemplate.swift 98.93% <100.00%> (+<0.01%) ⬆️
...ns/templates/query/FetchProjectQueryTemplate.swift 98.31% <100.00%> (+<0.01%) ⬆️

πŸ“£ We’re building smart automated test selection to slash your CI/CD build times. Learn more

@scottkicks scottkicks marked this pull request as ready for review February 22, 2023 22:11
Copy link
Contributor

@msadoon msadoon left a comment

Choose a reason for hiding this comment

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

Great work!

606F215A299D3EF800BA5CDF /* GraphAPI.TriggerCapiEventInput+TriggerCapiEventInputTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "GraphAPI.TriggerCapiEventInput+TriggerCapiEventInputTests.swift"; sourceTree = "<group>"; };
606F215D299D414800BA5CDF /* GraphAPI.TriggerCapiEventInput+TriggerCapiEventInput.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "GraphAPI.TriggerCapiEventInput+TriggerCapiEventInput.swift"; sourceTree = "<group>"; };
606F2165299D456D00BA5CDF /* TriggerCapiEvent.graphql */ = {isa = PBXFileReference; lastKnownFileType = text; path = TriggerCapiEvent.graphql; sourceTree = "<group>"; };
606F2167299D45F900BA5CDF /* TriggerCapiEventInput.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TriggerCapiEventInput.swift; sourceTree = "<group>"; };
Copy link
Contributor

@msadoon msadoon Feb 23, 2023

Choose a reason for hiding this comment

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

Weird that TriggerCapiEventInputInput showing multiple times...I checked the finder folder and xcode and the file is there only once.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok i will keep this in mind as I work through Part 2 and see if git is caching it for some reason.

@scottkicks scottkicks merged commit 06262b2 into main Feb 24, 2023
@scottkicks scottkicks deleted the web-970 branch February 24, 2023 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants