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

feat: support iteration fields #95

Merged
merged 5 commits into from
Oct 30, 2023
Merged

feat: support iteration fields #95

merged 5 commits into from
Oct 30, 2023

Conversation

blombard
Copy link
Contributor

Following the issue #94 I created, here is the start of solution.

I'm not used to Typescript so sorry if I make rookie mistakes. :)

Closes #94

@gr2m gr2m mentioned this pull request Feb 17, 2023
2 tasks
@blombard blombard closed this Feb 26, 2023
@blombard blombard reopened this Feb 26, 2023
@gr2m gr2m marked this pull request as ready for review May 20, 2023 01:24
Copy link
Owner

@gr2m gr2m left a comment

Choose a reason for hiding this comment

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

Changes look good, I can take care of the test conflicts

@gr2m
Copy link
Owner

gr2m commented May 20, 2023

I rebased your PR and removed updated fixtures for now.

When I run

node test/recorded/record-fixtures.js api.items.update-iteration

I get an error

TypeError: Cannot read properties of undefined (reading 'existsInProject')

Can you check that please?

@gr2m
Copy link
Owner

gr2m commented Oct 29, 2023

could you check the failing tests @blombard? Do run a single recorded test you can e.g. do

npx ava test/recorded.test.js --match api.items.add-draft-with-invalid-date

There have been a few changes since, it might be enough to re-record

@blombard
Copy link
Contributor Author

blombard commented Oct 29, 2023

Hi @gr2m, sorry for letting this fall into limbo.

Tests are now successful on my side.

Capture d’écran 2023-10-29 à 22 26 26

I see that there are now new conflicts but when I try to resolve them, new testing errors arise 😬
EDIT: Nevermind, it was only the recorded.test.js.snap conflict 👍
Capture d’écran 2023-10-29 à 22 47 20

# Conflicts:
#	test/snapshots/recorded.test.js.snap
Copy link
Owner

@gr2m gr2m left a comment

Choose a reason for hiding this comment

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

I just have one nit, but we can address it in a separate pull request to unblock folks who wait for the iteration field support

@@ -129,6 +129,28 @@ export function projectFieldsNodesToFieldsMap(state, project, nodes) {
);
}

// If the field is of type "Iteration", then the `configuration` property will be set.
if (node.configuration) {
Copy link
Owner

Choose a reason for hiding this comment

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

I wonder if we maybe should check for node.configuration?.iterations to be truthy? Just in case the .configuration will be used by other fields in future?

@gr2m gr2m changed the title Handle iterations field feat: support iteration fields Oct 30, 2023
@gr2m gr2m merged commit e581c3d into gr2m:main Oct 30, 2023
4 checks passed
@gr2m
Copy link
Owner

gr2m commented Oct 30, 2023

@all-contributors please add @blombard for code and test

@allcontributors
Copy link
Contributor

@gr2m

I've put up a pull request to add @blombard! 🎉

@github-actions
Copy link

🎉 This PR is included in version 5.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle iterations field
2 participants