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

Remove submitted task from task list #41

Merged
merged 1 commit into from Apr 27, 2023

Conversation

nbarnabee
Copy link
Collaborator

@nbarnabee nbarnabee commented Mar 8, 2023

This PR addresses T331269

Upon a "successful" task submission (e.g., one that does not return an error on the front end), the submitted task will be removed from the task array and a new one will be presented.

Because I needed to emit from the UserContributionForm component, I initially removed the Home.vue component entirely and stuck its code inside HomeView.vue.

On reflection, I decided that that was too radical a change and restored Home.vue, but set it up so that it is UserContributionForm's sibling rather than its parent. Both components take information about the current task from HomeView.vue.

@nbarnabee
Copy link
Collaborator Author

nbarnabee commented Mar 8, 2023

Hmm, except that now the tests are failing. Argh.

Oh, I see why. The tests are written assuming that "Home" will be fed a task...

@nbarnabee nbarnabee force-pushed the remove-submitted-task branch 2 times, most recently from 9992e68 to 308d063 Compare March 8, 2023 16:07
@nbarnabee
Copy link
Collaborator Author

I put my finger on the scale of that test pretty firmly. Ideally, I'd write a test that would load HomeView and cover the entire throughput, and I might have time to do that this evening.

Copy link
Collaborator

@Damilare1 Damilare1 left a comment

Choose a reason for hiding this comment

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

Thanks for all the work on this @nbarnabee.

Looks like there's a lot happening on this PR. We need to discuss this process and would be good to have some good unit test support to it too. We can focus on this after the first deployment.

src/components/__tests__/Home.spec.js Outdated Show resolved Hide resolved
src/views/HomeView.vue Outdated Show resolved Hide resolved
src/components/Home.vue Outdated Show resolved Hide resolved
src/components/Home.vue Outdated Show resolved Hide resolved
@nbarnabee
Copy link
Collaborator Author

Something seems to have gone funny when I merged the changes from main into this branch; I'm getting warnings that I didn't before and haven't yet gotten to the bottom of it, though everything still seems to be rendering correctly.

@nbarnabee
Copy link
Collaborator Author

b3895cc addresses comments made by Damilare. I still haven't resolved the warnings I alluded to in my earlier messages, but it looks like this change isn't going in prior to deployment anyway. I will have another look at it later.

@nbarnabee nbarnabee marked this pull request as draft March 16, 2023 08:33
@nbarnabee
Copy link
Collaborator Author

Given the numerous changes to the frontend, this will probably require some re-working. (And I STILL haven't figured out what was causing those errors.) If anyone else would like to take a stab at this, feel free.

@nbarnabee
Copy link
Collaborator Author

Reopening this one. I merged all of the changes that had happened on the main branch since the last time I updated it, and am no longer getting the various warnings. Functionally, this does what it's supposed to.

I still do not have a full test of everything that passes from HomeView to the child components and don't have an ETA on that.

@nbarnabee nbarnabee marked this pull request as ready for review March 21, 2023 21:20
@nbarnabee nbarnabee force-pushed the remove-submitted-task branch 2 times, most recently from dfb6a33 to 9fe1ba9 Compare March 30, 2023 04:19
I've merged several commits into this one:
1. Refactor Home.vue and UserContributionForm.vue to be sibling
components
2. Move functionality from Home.vue to HomeView.vue (this allows the
parent component, HomeView, to communicate directly with
UserContributionForm)
3. Rewrite the test for the Home.vue component.
@nbarnabee
Copy link
Collaborator Author

Hey @Damilare1, I've fixed the merge conflicts.

@Damilare1 Damilare1 merged commit 11e683d into wikimedia:main Apr 27, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants