Skip to content

Conversation

@PVince81
Copy link
Contributor

Prevents "other" value to be deleted.
When appending custom value, put it above the "other" entry.

Fixes #24664

Please review @owncloud/javascript @icewind1991 @guruz @georgehrke @SergioBertolinSG @humblemumble

  • TEST: select "Other", leave field without entering, reopen dropdown: dropdown looks sane
  • TEST: select "Other", enter value. Value properly selected in dropdown (before/after page refresh)
  • TEST: custom quota appears above "Other" in dropdown (before/after page refresh)

Prevents "other" value to be deleted.
When appending custom value, put it above the "other" entry.
@PVince81 PVince81 added this to the 9.1-current milestone Jun 23, 2016
@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @MorrisJobke, @ringmaster and @blizzz to be potential reviewers

@SergioBertolinSG
Copy link
Contributor

SergioBertolinSG commented Jun 24, 2016

Entering non numbers makes it put a quote of 'B'. Entering negative numbers put a quote of '?'.
Those values should be not allowed. (return to default value instead would be better)

screen shot 2016-06-24 at 09 09 36

@PVince81
Copy link
Contributor Author

@SergioBertolinSG ok, I added some crude validation to make you happy. Please try again.

@SergioBertolinSG
Copy link
Contributor

A couple of remaining cases:
A number divided by zero like "600/0" puts 'B' in the dropdown.
Using float numbers with comma instead of dot puts 'B' as well. (avoid that format or allow it)

@PVince81
Copy link
Contributor Author

I didn't intend this PR to be about validation, but ok let me try and make the validation slightly better.
I do not want to write a parser that checks for the MB/GB value here.

@SergioBertolinSG
Copy link
Contributor

SergioBertolinSG commented Jun 24, 2016

I think just avoiding those 'B' values and not allow number/0 and XX,XX format can be enough for now.

@guruz
Copy link
Contributor

guruz commented Jun 27, 2016

👍
Tested a bit, seems to work.

@PVince81 PVince81 modified the milestones: 9.2, 9.1 Jul 1, 2016
@PVince81
Copy link
Contributor Author

PVince81 commented Jul 1, 2016

@SergioBertolinSG the problem is as follows:

The user can enter values like "123", "123MB" or "123 MB".
Basically it's a number probably followed by letters.

So far I just used parseInt(quota, 10) which can detect whether there is at least a number at the beginning and ignore the letters.

But now if you type "1/0" it also passes the test because it starts with a number.

If we want to make it accurate, we need to develop a regexp that matches the expected formats. I wanted to avoid going that far in this quick fix. Also I don't think people would enter such strange values by mistake.

@SergioBertolinSG
Copy link
Contributor

Float numbers using comma are very likely to be introduced by people using translated setups.
Anyway I'm fine with merging this and open an issue about improving that validation with a regexp as an enhancement.

@PVince81 PVince81 merged commit a35747b into master Jul 4, 2016
@PVince81 PVince81 deleted the users-fixotherquotadropdown branch July 4, 2016 14:13
PVince81 pushed a commit that referenced this pull request Jul 8, 2016
…opdown"

This reverts commit a35747b, reversing
changes made to a573b68.
DeepDiver1975 pushed a commit that referenced this pull request Jul 8, 2016
* Revert "Merge pull request #25240 from owncloud/remove-svg"

This reverts commit 8b8d2b6, reversing
changes made to a35747b.

* Revert "Merge pull request #25253 from owncloud/users-fixotherquotadropdown"

This reverts commit a35747b, reversing
changes made to a573b68.

* Revert "Merge pull request #25314 from owncloud/files_external-backends-config"

This reverts commit a573b68, reversing
changes made to 8147eef.

* Revert "Add all properties while creating a subscription (#25318)"

This reverts commit aaf4c30.

* Revert "Merge pull request #25276 from owncloud/delete-own-session-token"

This reverts commit e42ce62, reversing
changes made to aaf4c30.

* Revert "Merge pull request #25262 from owncloud/fed-sharing-error"

This reverts commit 027715f, reversing
changes made to e42ce62.
@lock
Copy link

lock bot commented Aug 5, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

quota option "other" description incorrectly replaced

5 participants