Skip to content

Conversation

@fzngagan
Copy link
Contributor

No description provided.

@fzngagan fzngagan requested a review from angusmcleod November 1, 2021 07:11
@fzngagan fzngagan marked this pull request as ready for review November 1, 2021 11:08
category_id: category.id
).first

expect(user.guardian.send(:wizard_user_can_create_topic_on_category?, topic)).to be_truthy
Copy link
Member

Choose a reason for hiding this comment

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

@fzngagan We're calling a private method here. Is that what we want to be testing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem here is, the Rspec config in discourse is quite lax so the can_create_topic? and similar methods mostly return true so can't rely on testing them.

action['type'] === 'create_topic'
end

submission_data = begin
Copy link
Member

Choose a reason for hiding this comment

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

@fzngagan This should be abstracted into a find_by_id method on CustomWizard::Submission.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, will make that change.

wizard.presence
end

def wizard_can_create_topic_on_category?(wizard, topic)
Copy link
Member

Choose a reason for hiding this comment

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

@fzngagan Can't we just check if the topic has a wizard_submission_id and if the user is the author of the post? If a topic has that field it had to have been created by a wizard right? Won't this always return true if that's present?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@angusmcleod
What if the given wizard can no longer create topics?

Copy link
Contributor Author

@fzngagan fzngagan Nov 2, 2021

Choose a reason for hiding this comment

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

Or say, what if the user no longer has access to the wizard?

@fzngagan
Copy link
Contributor Author

fzngagan commented Dec 8, 2021

New Spec:
If user owns the topic, already has reply & see permissions and the topic is created via a wizard, grant the user edit rights.

@fzngagan fzngagan marked this pull request as draft December 16, 2021 08:39
@fzngagan fzngagan marked this pull request as ready for review January 31, 2022 07:36
@angusmcleod angusmcleod self-requested a review January 31, 2022 08:03
Copy link
Member

@angusmcleod angusmcleod left a comment

Choose a reason for hiding this comment

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

@fzngagan Looks good 👍

@angusmcleod angusmcleod merged commit 5bbb36e into main Jan 31, 2022
@angusmcleod angusmcleod deleted the wizard-permissions branch January 31, 2022 08:04
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.

3 participants