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

fix:restored removed article to available articles #6060

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

shishiro26
Copy link
Contributor

NOTE: Please review the pull request process before opening your first PR: https://github.com/WikiEducationFoundation/WikiEduDashboard/blob/master/CONTRIBUTING.md#pull-request-process

What this PR does

This PR restores both self-assigned and admin-assigned articles back to the pool of available articles.
I have written a test case for the controller to ensure that the changes work as expected.

Screenshots

After:
https://github.com/user-attachments/assets/db77ecb3-734d-4693-94d2-cfd235297c90

Open questions and concerns

With this change, when removing an article, a dialog appears with the message: "Are you sure you want to delete this program?"

  • Do we need to modify the dialog message to reflect the new functionality?

#5549

@shishiro26 shishiro26 marked this pull request as draft December 13, 2024 14:27
@shishiro26
Copy link
Contributor Author

In this PR, I have added Ruby test cases for the unclaim method in the AssignmentsController.

The scenarios covered are as follows:

  1. When the article is assigned to the user by the admin:
  2. When the article is assigned to the user by the admin:
  3. When the article is self-assigned:

@shishiro26 shishiro26 marked this pull request as ready for review December 23, 2024 15:45
@shishiro26
Copy link
Contributor Author

@ragesoss the test is passing locally in my system
image

@shishiro26
Copy link
Contributor Author

@ragesoss, I'm not entirely sure, but the multiwiki_assignment_spec.rb test seems to be failing after the latest commit merge. Additionally, the account_requests_spec.rb test is failing both on my local setup and in the master branch. However, this failure appears to be unrelated to the changes I made.

@ragesoss
Copy link
Member

@shishiro26 yes, i'm not sure why it started failing, but i'm going to look into it.

@ragesoss
Copy link
Member

Does this implementation differentiate articles that started as Available Articles versus ones that the student chose via title without starting as an Available Article? I think we need this to only apply for ones that started as Available Articles.

@shishiro26
Copy link
Contributor Author

shishiro26 commented Dec 31, 2024

I’m not sure if you’re referring to this one, sir

User:
https://github.com/user-attachments/assets/0cfa08ac-d75f-4f52-a223-a3c63a471ec9

If so, then yes, it doesn’t differentiate between the two. It removes both from the assigned ones and keeps them in the available articles, sir.

@shishiro26
Copy link
Contributor Author

Sir, I would like to know if there is any user-specific association, such as a user_id or something similar, that tracks whether a particular person created or added an article. Is there an existing reference to identify the user who created or added the article? From the logs I’ve seen, there are references to users, but they seem to be related to assignments rather than article creation. If such an association doesn’t currently exist, would it be possible for me to add a user_id field to the articles table and link it to the user who created or added the article?

@shishiro26 shishiro26 marked this pull request as draft January 2, 2025 16:59
@ragesoss
Copy link
Member

ragesoss commented Jan 2, 2025

I think you could use Assignment#flags to keep track of whether an Assignment record began as an Available Article.

@shishiro26
Copy link
Contributor Author

Sir, I believe there is no change in the flags field, as it remains NULL in both cases in the SQL database. There appears to be no update to Assignment.flags.

@ragesoss
Copy link
Member

ragesoss commented Jan 2, 2025

Right, you would need to implement that.

@shishiro26
Copy link
Contributor Author

okay, sir! 😄

@shishiro26
Copy link
Contributor Author

Could you please review this, sir? This is just a rough implementation. If this method looks fine, the unclaim method (which removes the article if it is not in the available articles pool, and if it is in the pool, the assignment is simply unclaimed and added back to the available articles), and if the assignment#flags are set correctly, I will refine the code and update the RSpec tests for the final version.

@ragesoss
Copy link
Member

ragesoss commented Jan 3, 2025

I will plan to review this next week.

@shishiro26
Copy link
Contributor Author

Okay sir

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.

2 participants