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.

Sign up for GitHub

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

Enforce unique Rules (Old Urls) #62

Closed
wants to merge 1 commit into from
Closed

Enforce unique Rules (Old Urls) #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions ...ules/_protected/Forte.EpiserverRedirects/ClientResources/RedirectsMenu/RedirectsMenu.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,18 @@ <h2 class="redirect-importer-header">Import redirects from CSV file</h2>
<button data-dojo-type="dijit/form/Button" data-dojo-attach-point="deleteButton" type="button">Delete</button>
<button data-dojo-type="dijit/form/Button" data-dojo-attach-point="refreshButton" type="button">Refresh</button>
<button data-dojo-type="dijit/form/Button" data-dojo-attach-point="clearAllButton" type="button">Clear all</button>
<!-- <div class="epi-formArea">
<label>Simulate Wildcard redirect:</label>
<input class="form-input" type="text" name="simulateOldUrl"
data-dojo-props="intermediateChanges:true"
data-dojo-type="dijit/form/TextBox"
placeHolder="Old Url"
data-dojo-attach-point="simulateOldUrlTextBox" id="simulateOldUrl" />
<button data-dojo-type="dijit/form/Button" data-dojo-attach-point="simulateFindButton" type="button">Find</button>
<button data-dojo-type="dijit/form/Button" data-dojo-attach-point="simulateResetButton" type="button">Reset</button>
</div>-->
<button data-dojo-type="dijit/form/Button" data-dojo-attach-point="removeDuplicatesButton" type="button">Remove duplicates</button>
<span name="removeDuplicatesStatus" value="" data-dojo-attach-point="removeDuplicatesStatus"></span>
<!-- <div class="epi-formArea">
<label>Simulate Wildcard redirect:</label>
<input class="form-input" type="text" name="simulateOldUrl"
data-dojo-props="intermediateChanges:true"
data-dojo-type="dijit/form/TextBox"
placeHolder="Old Url"
data-dojo-attach-point="simulateOldUrlTextBox" id="simulateOldUrl" />
<button data-dojo-type="dijit/form/Button" data-dojo-attach-point="simulateFindButton" type="button">Find</button>
<button data-dojo-type="dijit/form/Button" data-dojo-attach-point="simulateResetButton" type="button">Reset</button>
</div>-->
<div class="url-redirect-grid" data-dojo-type="redirectsMenu-grid/RedirectsMenuGrid" data-dojo-attach-point="redirectsMenuGrid">
</div>
</div>
Expand Down
12 changes: 12 additions & 0 deletions ...odules/_protected/Forte.EpiserverRedirects/ClientResources/RedirectsMenu/RedirectsMenu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ define("redirectsMenu/RedirectsMenu", [
on(this.deleteButton, "click", this._onDeleteClick.bind(this));
on(this.refreshButton, "click", this._refreshView.bind(this));
on(this.clearAllButton, "click", this._onClearAllClick.bind(this));
on(this.removeDuplicatesButton, "click", this._onRemoveDuplicatesClick.bind(this));
/*on(this.simulateFindButton, "click", this._onSimulateFindClick.bind(this));
on(this.simulateResetButton, "click", this._onSimulateResetClick.bind(this));*/
on(this.uploadFormSubmit, "click", this._onImportSubmit.bind(this));
Expand Down Expand Up @@ -130,6 +131,17 @@ define("redirectsMenu/RedirectsMenu", [
}
},

_onRemoveDuplicatesClick: function() {
if (window.confirm("Do you really want to remove all duplicates?")) {
var removeDuplicatesStatus = this.removeDuplicatesStatus;
removeDuplicatesStatus.innerText = "Removing duplicates...";
this.redirectsMenuViewModel.removeDuplicateRules().then((r) => {
removeDuplicatesStatus.innerText = "Duplicates removed.";
this._refreshView()
});
}
},

_onEditClick: function () {
if (this.selectedModel.redirectRuleType === "System") return;

Expand Down
6 changes: 6 additions & 0 deletions ...rotected/Forte.EpiserverRedirects/ClientResources/RedirectsMenu/RedirectsMenuViewModel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
], function (declare, Stateful, dependency) {

var clearAllGuid = "00000000-0000-0000-0000-000000000000";
var clearAllDuplicatesGuid = "00000000-0000-0000-0000-000000000001";

return declare([Stateful], {
store: null,

Expand Down Expand Up @@ -64,6 +66,10 @@

clearRedirectRules: function() {
return this.store.remove(clearAllGuid);
},

removeDuplicateRules: function () {
return this.store.remove(clearAllDuplicatesGuid);
}
});
});
22 changes: 12 additions & 10 deletions EpiserverRedirects/ClientResources/RedirectsMenu/RedirectsMenu.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,18 @@ <h2 class="redirect-importer-header">Import redirects from CSV file</h2>
<button data-dojo-type="dijit/form/Button" data-dojo-attach-point="deleteButton" type="button">Delete</button>
<button data-dojo-type="dijit/form/Button" data-dojo-attach-point="refreshButton" type="button">Refresh</button>
<button data-dojo-type="dijit/form/Button" data-dojo-attach-point="clearAllButton" type="button">Clear all</button>
<!-- <div class="epi-formArea">
<label>Simulate Wildcard redirect:</label>
<input class="form-input" type="text" name="simulateOldUrl"
data-dojo-props="intermediateChanges:true"
data-dojo-type="dijit/form/TextBox"
placeHolder="Old Url"
data-dojo-attach-point="simulateOldUrlTextBox" id="simulateOldUrl" />
<button data-dojo-type="dijit/form/Button" data-dojo-attach-point="simulateFindButton" type="button">Find</button>
<button data-dojo-type="dijit/form/Button" data-dojo-attach-point="simulateResetButton" type="button">Reset</button>
</div>-->
<button data-dojo-type="dijit/form/Button" data-dojo-attach-point="removeDuplicatesButton" type="button">Remove duplicates</button>
<span name="removeDuplicatesStatus" value="" data-dojo-attach-point="removeDuplicatesStatus"></span>
<!-- <div class="epi-formArea">
<label>Simulate Wildcard redirect:</label>
<input class="form-input" type="text" name="simulateOldUrl"
data-dojo-props="intermediateChanges:true"
data-dojo-type="dijit/form/TextBox"
placeHolder="Old Url"
data-dojo-attach-point="simulateOldUrlTextBox" id="simulateOldUrl" />
<button data-dojo-type="dijit/form/Button" data-dojo-attach-point="simulateFindButton" type="button">Find</button>
<button data-dojo-type="dijit/form/Button" data-dojo-attach-point="simulateResetButton" type="button">Reset</button>
</div>-->
<div class="url-redirect-grid" data-dojo-type="redirectsMenu-grid/RedirectsMenuGrid" data-dojo-attach-point="redirectsMenuGrid">
</div>
</div>
Expand Down
12 changes: 12 additions & 0 deletions EpiserverRedirects/ClientResources/RedirectsMenu/RedirectsMenu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ define("redirectsMenu/RedirectsMenu", [
on(this.deleteButton, "click", this._onDeleteClick.bind(this));
on(this.refreshButton, "click", this._refreshView.bind(this));
on(this.clearAllButton, "click", this._onClearAllClick.bind(this));
on(this.removeDuplicatesButton, "click", this._onRemoveDuplicatesClick.bind(this));
/*on(this.simulateFindButton, "click", this._onSimulateFindClick.bind(this));
on(this.simulateResetButton, "click", this._onSimulateResetClick.bind(this));*/
on(this.uploadFormSubmit, "click", this._onImportSubmit.bind(this));
Expand Down Expand Up @@ -130,6 +131,17 @@ define("redirectsMenu/RedirectsMenu", [
}
},

_onRemoveDuplicatesClick: function() {
if (window.confirm("Do you really want to remove all duplicates?")) {
var removeDuplicatesStatus = this.removeDuplicatesStatus;
removeDuplicatesStatus.innerText = "Removing duplicates...";
this.redirectsMenuViewModel.removeDuplicateRules().then((r) => {
removeDuplicatesStatus.innerText = "Duplicates removed.";
this._refreshView()
});
}
},

_onEditClick: function () {
if (this.selectedModel.redirectRuleType === "System") return;

Expand Down
6 changes: 6 additions & 0 deletions EpiserverRedirects/ClientResources/RedirectsMenu/RedirectsMenuViewModel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
], function (declare, Stateful, dependency) {

var clearAllGuid = "00000000-0000-0000-0000-000000000000";
var clearAllDuplicatesGuid = "00000000-0000-0000-0000-000000000001";

return declare([Stateful], {
store: null,

Expand Down Expand Up @@ -64,6 +66,10 @@

clearRedirectRules: function() {
return this.store.remove(clearAllGuid);
},

removeDuplicateRules: function () {
return this.store.remove(clearAllDuplicatesGuid);
}
});
});
15 changes: 12 additions & 3 deletions EpiserverRedirects/Menu/RedirectRuleStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public class RedirectRuleStore : RestControllerBase
private readonly IRedirectRuleRepository _redirectRuleRepository;
private readonly IRedirectRuleMapper _redirectRuleMapper;
private readonly Guid _clearAllGuid = Guid.Parse("00000000-0000-0000-0000-000000000000");
private readonly Guid _clearAllDuplicatesGuid = Guid.Parse("00000000-0000-0000-0000-000000000001");

public RedirectRuleStore(IRedirectRuleRepository redirectRuleRepository, IRedirectRuleMapper redirectRuleMapper)
{
Expand Down Expand Up @@ -77,9 +78,17 @@ public ActionResult Put(RedirectRuleDto dto)
[HttpDelete]
public ActionResult Delete(Guid id)
{
var deletedSuccessfully = id == _clearAllGuid
? _redirectRuleRepository.ClearAll()
: _redirectRuleRepository.Delete(id);
bool deletedSuccessfully;

if (id == _clearAllGuid) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not fluent with EpiServer store concept but I wonder if this is possible to have separate methods in controller (I believe it needs to be supported by JS side of EpiServer store): Delete(GuidId), ClearAll() and RemoveAllDuplicates() ?

deletedSuccessfully = _redirectRuleRepository.ClearAll();
}
else if(id == _clearAllDuplicatesGuid) {
deletedSuccessfully = _redirectRuleRepository.RemoveAllDuplicates();
}
else {
deletedSuccessfully = _redirectRuleRepository.Delete(id);
}

return deletedSuccessfully
? Rest(HttpStatusCode.OK)
Expand Down
46 changes: 45 additions & 1 deletion EpiserverRedirects/Repository/DynamicDataStoreRepository.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,31 @@ public class DynamicDataStoreRepository : RedirectRuleRepository
private readonly DynamicDataStoreFactory _dynamicDataStoreFactory;
private DynamicDataStore DynamicDataStore => _dynamicDataStoreFactory.CreateStore(typeof(RedirectRule));

private IQueryable<RedirectRule> GetAllByOldPattern(string oldPattern)
{
return DynamicDataStore.Items<RedirectRule>().Where(x => x.OldPattern == oldPattern);
}

private IQueryable<RedirectRule> RemoveDuplicates(RedirectRule redirectRule)
{
var oldPatternRedirectRules =
GetAllByOldPattern(redirectRule.OldPattern)
.Where(x => x.RedirectRuleType != RedirectRuleType.Regex)
.OrderByDescending(x => x.CreatedOn);

IQueryable<RedirectRule> duplicates = null;

if (oldPatternRedirectRules.Count() > 1)
{
duplicates = oldPatternRedirectRules.Skip(1);

foreach (var duplicate in duplicates)
{
Delete(duplicate.Id.ExternalId);
}
}
return duplicates;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this method returns IQueryable<RedirectRule> (which I'm not 100% sure is required, since it is not used anywhere) I think to be on safe side we should check for null here so the last line:
return duplicates ?? Enumerable.Empty<RedirectRule>().AsQueryable();
so we do not hit NullReferenceException by accident

}
public DynamicDataStoreRepository(DynamicDataStoreFactory dynamicDataStoreFactory)
{
_dynamicDataStoreFactory = dynamicDataStoreFactory;
Expand All @@ -27,8 +51,10 @@ public override RedirectRule GetById(Guid id)
}

public override RedirectRule Add(RedirectRule redirectRule)
{
{
DynamicDataStore.Save(redirectRule);
RemoveDuplicates(redirectRule);

return redirectRule;
}

Expand All @@ -46,6 +72,24 @@ public override RedirectRule Update(RedirectRule redirectRule)
return redirectRuleToUpdate;
}

public override bool RemoveAllDuplicates()
{
var redirectRules = GetAll();

try
{
foreach (var redirectRule in redirectRules)
{
RemoveDuplicates(redirectRule);
}
return true;
}
catch
{
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel that at least what we should do here is to log error through ILogger - I know errors are ignored in every other method (ie Delete, ClearAll) but maybe we should fix that? What do you think @m-jedynak
(Even better solution is to return errror message to end user)

}
}

public override bool Delete(Guid id)
{
try
Expand Down
2 changes: 2 additions & 0 deletions EpiserverRedirects/Repository/RedirectRuleCachedRepositoryDecorator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ public IQueryable<RedirectRule> GetAll()

public RedirectRule Update(RedirectRule redirectRule) => _redirectRuleRepository.Update(redirectRule);

public bool RemoveAllDuplicates() => _redirectRuleRepository.RemoveAllDuplicates();

public bool Delete(Guid id) => _redirectRuleRepository.Delete(id);

public bool ClearAll() => _redirectRuleRepository.ClearAll();
Expand Down
2 changes: 2 additions & 0 deletions EpiserverRedirects/Repository/Repository.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public interface IRedirectRuleRepository
IQueryable<RedirectRule> GetAll();
RedirectRule Add(RedirectRule redirectRule);
RedirectRule Update(RedirectRule redirectRule);
bool RemoveAllDuplicates();
bool Delete(Guid id);
bool ClearAll();
}
Expand All @@ -20,6 +21,7 @@ public abstract class RedirectRuleRepository : IRedirectRuleRepository
public abstract IQueryable<RedirectRule> GetAll();
public abstract RedirectRule Add(RedirectRule redirectRule);
public abstract RedirectRule Update(RedirectRule redirectRule);
public abstract bool RemoveAllDuplicates();
public abstract bool Delete(Guid id);
public abstract bool ClearAll();

Expand Down