Skip to content

Default article rename bugfix #338

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

Merged
merged 5 commits into from
Nov 6, 2018

Conversation

danielmpetrov
Copy link
Contributor

@danielmpetrov danielmpetrov commented Oct 30, 2018

The issue

To my understanding, the default article is special to CoreWiki - it serves as a home page, and cannot be deleted.

However, it can be renamed, and this causes unwanted functionality to be revealed.

  • A Back to home button appears when you're already on the home page
  • The Delete button appears, enabling the admin to delete this special article, causing CoreWiki home page to return 404

The cause

The reason for this bug is that we're performing a slug check against a C# constant 'home-page', but the actual slug changes when the topic is modified.

Furthermore, this same check is duplicated across Pages and PageModels.

Proposed solution

The concept that an article is a default home page is something specific to our domain, so I have added an IsHomePage property to our Domain Article. For the time being it returns true if the article id is 1, which resolves the issues of 'Back to home' and 'Delete' buttons appearing when they should not.

In addition, by adding an IsHomePage property to the necessary Dtos, and letting AutoMapper take care of the rest, we are enabling ourselves to control the logic of identifying the default article in a single place - the domain object. 🎉

@danielmpetrov danielmpetrov force-pushed the bugfix_default_article branch from 020da89 to 775f209 Compare October 31, 2018 17:51
@danielmpetrov danielmpetrov force-pushed the bugfix_default_article branch from b03ff71 to c23b036 Compare November 1, 2018 08:11
@csharpfritz csharpfritz merged commit a952671 into csharpfritz:dev Nov 6, 2018
@csharpfritz
Copy link
Owner

This is really cool... and has started a good conversation around protecting articles and configuring the home page

@danielmpetrov danielmpetrov deleted the bugfix_default_article branch November 7, 2018 07:16
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