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

Auto-accept remote deletes #90

Merged
merged 5 commits into from
Oct 30, 2024
Merged

Auto-accept remote deletes #90

merged 5 commits into from
Oct 30, 2024

Conversation

magush27
Copy link
Collaborator

hyphacoop/distributed-press-organizing#162

The code in apsystem.test doesn't work, what is the problem?

@akhileshthite
Copy link
Collaborator

hyphacoop/distributed-press-organizing#162

The code in apsystem.test doesn't work, what is the problem?

I think it's because of the merge conflicts, can you resolve those?

@RangerMauve
Copy link
Contributor

Yeah, please fix the conflict before we merge. Also, the CI is showing that apsystem.ts has unnecessary import for ActivityStore, please remove it so the linter can pass. You can also run the linter locally with npm run lint

https://github.com/hyphacoop/social.distributed.press/actions/runs/11520192999/job/32070934990

@fauno
Copy link
Collaborator

fauno commented Oct 28, 2024

solved the conflict and linted!

@RangerMauve
Copy link
Contributor

Tests seem to be failing 😅

@magush27
Copy link
Collaborator Author

Tests seem to be failing 😅

Yep, I had this problem before when I made the first merge with main on the tests file and I don't know what is the problem. I've checked the code and I'm using the activity as other tests do and the correct endpoints (I think hehe), have you got any idea of what is going on?

@RangerMauve
Copy link
Contributor

The issue was that the actor wasn't being mocked correctly.

Then there was an issue with the delete activity not being mocked by the mockFetch

Then there was an issue with the wrong activity ID being used in apsystem.ts. It was using the id of the Delete activity instead of the object the delete was pointing at

Then there was the unnecessary check that the inbox was being called at the end

Looks good to go now

Next time please try to get the tests passing before getting a review and if you're stuck writing down the steps you took to debug so far. 👍

@RangerMauve RangerMauve merged commit 5530ce7 into main Oct 30, 2024
2 checks passed
@magush27
Copy link
Collaborator Author

The issue was that the actor wasn't being mocked correctly.

Then there was an issue with the delete activity not being mocked by the mockFetch

Then there was an issue with the wrong activity ID being used in apsystem.ts. It was using the id of the Delete activity instead of the object the delete was pointing at

Then there was the unnecessary check that the inbox was being called at the end

Looks good to go now

Next time please try to get the tests passing before getting a review and if you're stuck writing down the steps you took to debug so far. 👍

Oh, I see. Thanks for the explanation and sorry!
Next time I'll get the tests passing before making the pr

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

Successfully merging this pull request may close these issues.

4 participants