tessus / mwExtensionMantis

MediaWiki Extension:Mantis
https://www.mediawiki.org/wiki/Extension:Mantis
GNU General Public License v2.0
2 stars 6 forks source link

Added possibility to filter for version and fixed_in_version #20

Closed pedwik closed 7 years ago

pedwik commented 7 years ago

Usage: Alias for fixed_in_version is "releaseversion", maybe not thought that through. Anyhow, if a parameter "releaseversion = 2.7.1," (last entry must be followed by a comma) then only records with b.fixed_in_version = 2.7.1 are selected. If parameter "version" is used instead, records with b.version are selected. Example:

releaseversion = 2.7.1, version = 2.7.1,
tessus commented 7 years ago

Thanks for the PR, but it has a few problems:

I'll use https://github.com/HSINWEI/mwExtensionMantis/commit/4ecbc7062da3d6d3e9e713e255630cba5a92c652 as a start and go from there. I'll have time this evening and look into it. The problem is not the coding, but writing the freaking documentation.

pedwik commented 7 years ago

Ok, the comma was not my intent, it should be fixed. Risk for SQL Injection should be eliminated if we add a constraint that the first argument character must be a number.

2017-09-06 19:38 GMT+02:00 Helmut K. C. Tessarek notifications@github.com:

Thanks for the PR, but it has a few problems:

  • the naming (as you mentioned) is not ok
  • why does the last entry has to be followed by a comma? this is not very user friendly.
  • it allows SQL injection

I'll use HSINWEI/mwExtensionMantis@4ecbc70 https://github.com/HSINWEI/mwExtensionMantis/commit/4ecbc7062da3d6d3e9e713e255630cba5a92c652 as a start and go from there. I'll have time this evening and look into it. The problem is not the coding, but writing the freaking documentation.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tessus/mwExtensionMantis/pull/20#issuecomment-327558773, or mute the thread https://github.com/notifications/unsubscribe-auth/AMUIM1A-m4IdYtc4GGKD3-gF4Os6HZgKks5sfth8gaJpZM4POs4V .

tessus commented 7 years ago

I'll be off work in about 5 hours. I'll look into it then. But as I mentioned I will use this commit as a start -> https://github.com/HSINWEI/mwExtensionMantis/commit/4ecbc7062da3d6d3e9e713e255630cba5a92c652

I'm closing this PR. Please open a ticket 'support version and fixed_in_version'.

pedwik commented 7 years ago

An even more flexible filtering function for version number would allow "less than" etc, but it is a bit work to do that in a sql query, although doable.

2017-09-06 20:02 GMT+02:00 Helmut K. C. Tessarek notifications@github.com:

I'll be off work in about 5 hours. I'll look into it then. But as I mentioned I will use this commit as a start -> HSINWEI/mwExtensionMantis@ 4ecbc70 https://github.com/HSINWEI/mwExtensionMantis/commit/4ecbc7062da3d6d3e9e713e255630cba5a92c652

I'm closing this PR. Please open a ticket 'support version and fixed_in_version'.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tessus/mwExtensionMantis/pull/20#issuecomment-327565720, or mute the thread https://github.com/notifications/unsubscribe-auth/AMUIM3skN8RwzupK_NfRhCUsnzx72UhTks5sft5DgaJpZM4POs4V .

tessus commented 7 years ago

Hmm, think about this for a while. How would you do this, if you allowed a list of version numbers? You have to find an additional logic that solves a bunch of problems. Translating into SQL is not the problem.

What do you do if you encounter 2 operators (not the value) that are the same e.g. <? What if expressions contradict each other? How to combine several operators (with AND or OR)? I would have to build an expression evaluator into the code, because I can't just send something to the database backend.

Unless you have an easy way to address these issues a flexible filter mechanism is out of the question.

tessus commented 7 years ago

Additional info: above examples are by far not the only cases that could occur.

pedwik commented 7 years ago

I do not think we have to overcomplicate things, but as a user I would appreciate defining a filter as for example

Version >= 2.71 for one wikitable,

and Version < 2.7.1 for another.

The wiki extension is in my case only a tool to be able to show for example bug fixes to a broader group of users, users that have no access to the MantisBT. I see no reason to implement a super-advanced parser for filtering versions.

Mvh Peder

6 sep. 2017 kl. 22:15 skrev Helmut K. C. Tessarek notifications@github.com:

Additional info: above examples are by far not the only cases that could occur.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

tessus commented 7 years ago

I've added new options (available on the master branch):

All of them allow a list of values (separated by comma)

Currently the *version* options cannot use operators like <, >, <=, >=

I have to think of a safe and easy way to handle these operators.

pedwik commented 7 years ago

Great Helmut! Concerning the operators, good old fortran-style operators like gt, ge, eq, lt, le could be used maybe?

2017-09-07 3:54 GMT+02:00 Helmut K. C. Tessarek notifications@github.com:

I've added new options (available on the master branch):

  • username
  • fixed_in_version alias: fixed_in
  • version
  • target_version alias: target

All of them allow a list of values (separated by comma)

Currently the version options cannot use operators like <, >, <=, >=

I have to think of a safe and easy way to handle these operators.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tessus/mwExtensionMantis/pull/20#issuecomment-327660360, or mute the thread https://github.com/notifications/unsubscribe-auth/AMUIM-W2jCHtT44aEms53vZxEoTCp3-bks5sf0zjgaJpZM4POs4V .

tessus commented 7 years ago

@pedwik done. available in master. see commit message for explanation.

Now I have to release 2.0 and write the freaking documentation... Argh.

tessus commented 7 years ago

sorry, wrong commit. that's the right one: 820ccb3f9ab72d2c2a09dada4bd64644242a92ec