Skip to content

Conversation

abbra
Copy link

@abbra abbra commented Feb 19, 2018

Since there are two different settings: allowAnonymous and
allowAnonymousEdits, only show new guest note button in the latter case.

Same for publishing a note: if allowAnonymousEdits is not enabled, it makes no
sense to allow publishing a revision.

Signed-off-by: Alexander Bokovoy ab@vda.li

Since there are two different settings: allowAnonymous and
allowAnonymousEdits, only show new guest note button in the latter case.

Same for publishing a note: if allowAnonymousEdits is not enabled, it makes no
sense to allow publishing a revision.

Signed-off-by: Alexander Bokovoy <ab@vda.li>
@abbra abbra mentioned this pull request Feb 19, 2018
useCDN: config.usecdn
useCDN: config.usecdn,
allowAnonymousEdits: false,
signin: false
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't these newly added options get a useful value instead of being statically set to false?

Copy link
Author

Choose a reason for hiding this comment

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

in their context we only have a response object, res. If that object can allow them to be derived from something -- fine. I was not able to find out what I can derive them from.

As this is just about displaying an error message, I think it is OK to not display the edit/publish buttons but we have to have variables defined to avoid throwing an error in the EJS code that references them.

@SISheogorath SISheogorath added the enhancement Wants to improvide an existing feature label Feb 26, 2018
@SISheogorath
Copy link
Contributor

After testing it and revisiting what the option allowanonymousedits was created for (#690) I think this PR breaks this purpose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Wants to improvide an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants