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

Make sure end date is after start date in Crowdfund app #4084

Merged

Conversation

bolatovumar
Copy link
Contributor

@bolatovumar bolatovumar commented Aug 28, 2022

I noticed we don't validate that end date is after start date in the crowdfund app. We should probably check for this. This PR adds this check.

Capture

@@ -374,7 +380,13 @@ private async Task<string> GetStoreDefaultCurrentIfEmpty(string storeId, string
{
if (string.IsNullOrWhiteSpace(currency))
{
currency = (await _storeRepository.FindStore(storeId)).GetStoreBlob().DefaultCurrency;
var store = await _storeRepository.FindStore(storeId);
if (store == null)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

store can be null here technically so we need to check for that.

</div>
<span asp-validation-for="StartDate" class="text-danger"></span>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Placed these below both of the elements so the layout doesn't get messed up if there is an error.

@@ -287,12 +288,17 @@ public async Task<IActionResult> UpdateCrowdfund(string appId, UpdateCrowdfundVi

if (Enum.Parse<CrowdfundResetEvery>(vm.ResetEvery) != CrowdfundResetEvery.Never && !vm.StartDate.HasValue)
{
ModelState.AddModelError(nameof(vm.StartDate), "A start date is needed when the goal resets every X amount of time.");
ModelState.AddModelError(nameof(vm.StartDate), "A start date is needed when the goal resets every X amount of time");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed a period character at the end since other errors don't have it.

}

if (Enum.Parse<CrowdfundResetEvery>(vm.ResetEvery) != CrowdfundResetEvery.Never && vm.ResetEveryAmount <= 0)
{
ModelState.AddModelError(nameof(vm.ResetEveryAmount), "You must reset the goal at a minimum of 1 ");
ModelState.AddModelError(nameof(vm.ResetEveryAmount), "You must reset the goal at a minimum of 1");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was an extra trailing space here which I removed.

@bolatovumar
Copy link
Contributor Author

Failing test unrelated

Capture

Copy link
Member

@dennisreimann dennisreimann left a comment

Choose a reason for hiding this comment

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

tACK

@dstrukt
Copy link
Member

dstrukt commented Aug 30, 2022

tACK, nice catch

@dstrukt dstrukt self-requested a review August 30, 2022 01:19
@NicolasDorier NicolasDorier merged commit 7106830 into btcpayserver:master Sep 9, 2022
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.

None yet

4 participants