Re: [pgadmin-hackers] [pgAdmin4] [PATCH] History Tab rewrite in React

[Dave Page]

On Thu, May 18, 2017 at 4:37 PM, Matthew Kleiman  wrote:
> Dave,
>
> Can you try running grunt tests on the command line?
>
> The tests task will run the eslint and karma grunt tasks, which should
> automatically webpack the React components before running the jasmine tests
> in karma.
>
> If it still fails, send us the output of that.

(pgadmin4)piranha:web dpage$ grunt tests
Running "eslint:target" (eslint) task

Running "karma:unit" (karma) task
19 05 2017 15:26:13.366:ERROR [plugin]: Error during loading
"karma-phantomjs-launcher" plugin:
  Path must be a string. Received null

webpack: Compiled successfully.
webpack: Compiling...
webpack: wait until bundle finished:
webpack: wait until bundle finished:
webpack: wait until bundle finished:
webpack: wait until bundle finished:
webpack: wait until bundle finished:
webpack: wait until bundle finished:
webpack: wait until bundle finished:
webpack: wait until bundle finished:
webpack: wait until bundle finished:
webpack: wait until bundle finished:
webpack: wait until bundle finished:
webpack: wait until bundle finished:
(node:66274) DeprecationWarning: loaderUtils.parseQuery() received a
non-string value which can be problematic, see
https://github.com/webpack/loader-utils/issues/56
parseQuery() will be replaced with getOptions() in the next major
version of loader-utils.
19 05 2017 15:26:18.887:WARN [karma]: No captured browser, open
http://localhost:9876/

webpack: Compiled successfully.
19 05 2017 15:26:18.895:ERROR [karma]: Found 1 load error
Warning: Task "karma:unit" failed. Use --force to continue.

Aborted due to warnings.


:-(

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] Declarative partitioning in pgAdmin4

[Dave Page]

On Thu, May 18, 2017 at 11:41 AM, Akshay Joshi <
akshay.jo...@enterprisedb.com> wrote:

> Hi All
>
> I have started implementation for Declarative Partitioning in pgAdmin4.
> Following are the tasks that I have implemented till now:
>
>- Show partitioned table and it's partitions under the parent table.
>Refer Partitioned_Table.png
>- To implement above I have created 'partitions' collection node and
>'partition' node under table node which is nothing but table node itself.
>To reduce redundant/duplicate code I have made following changes:
>   - Create new file "*utils.py*" under tables folder. Create a new
>   class BaseTableView(PGChildNodeView): derived from PGChildNodeView.
>   TableView and PartitionsView (new class for partition table) is
>   derived from BaseTableView.
>   - Move the common logic like dependencies, dependents, reversed
>   engineered sql, statistics, reset statistics in BaseTableView class
>   functions and then call that function from derived class like
>   BaseTableView.get_table_dependencies(self, tid)
>   - Will move more generic logic as we progress on this task.
>- Updated supported nodes list in DataGrid(View Data),
>Backup, Maintenance, Restore to show context menu for partitions.
>- Make sure dependencies, dependents, statistics, truncate,
>delete/drop and Reset Statistics works with partitions.
>- Updated jinja template to show correct reversed engineered sql for
>partitioned table. Please refer the "List_with_expression.png" for
>List partition and "Range_with_column_expression.png" for Range
>partition.
>- Updated jinja template to show correct sql for partitions of parent
>table. Please refer "SQL_Range_Partitions.png" and "SQL
>_List_Partitions.png". Some R is still require for other syntax too.
>
> Please let me know above looks good and am I going in right direction.
>

Certainly looks like it to me. We may want to tweak some things based on
the work Shirley is doing, but I think we're on the right path.

Good work!


>
>
> On Thu, May 11, 2017 at 7:06 PM, Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>> Hi Dave
>>
>> On Thu, May 11, 2017 at 6:54 PM, Dave Page 
>> wrote:
>>
>>>
>>>
>>> On Thu, May 11, 2017 at 11:35 AM, Akshay Joshi <
>>> akshay.jo...@enterprisedb.com> wrote:
>>>
 Hi

 On Thu, May 4, 2017 at 4:00 PM, Dave Page  wrote:

> Hi
>
> On Thu, May 4, 2017 at 10:29 AM, Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>> Hi All
>>
>> On Wed, May 3, 2017 at 5:35 PM, Dave Page  wrote:
>>
>>> Great, thanks.
>>>
>>> I think it's clear that we need to display the child partitions in
>>> the treeview. I don't see any other sensible way of enabling those
>>> operations without an extremely contrived dialogue design.
>>>
>>> Please now document how those features will be implemented; e.g, for
>>> each one:
>>>
>>> - View table data: Parent and partition context menu.
>>> - Attach/detach partitions: Parent properties dialogue
>>> ...
>>>
>>> That will then give us a list of places we'll need to (re)design
>>> dialogues and menus etc. for.
>>>
>>
>> As per my knowledge on Partitioning, I think we will have to
>> implement following things in parent and child:
>>
>>Parent:
>>
>>1. View Table data :  No need to change any logic, it's working.
>>2. Correct jinja template to show correct SQL in SQL pane.
>>3. Create partitioned table -
>>   - Add one switch control ("Partitioned Table?") in General tab
>>   of Table dialog.
>>   - Add new tab "Partitions".
>>   - Add one select2 control (Partition Type :Range/List) in
>>   "Partitions" tab.
>>   - Create one subnode control to specify number of key columns
>>   with expressions. For List partition only one row will be there + 
>> button
>>   will be disabled, and for Range partition + button will be 
>> enabled. Here is
>>   the syntax as per documentation [ PARTITION BY { RANGE | LIST
>>   } ( { *column_name* | ( *expression* ) } [ COLLATE *collation* ]
>>   [ *opclass* ] [, ... ] ) ]. *Design discussion required here
>>   for how user will specify expression, collate and opclass*.
>>4. Create N number of partitions:
>>   - Design one control (subnode control) so that user will add N
>>   number of partitions. Here is the syntax as per documentation 
>> CREATE
>>TABLE  *table_name PARTITION OF parent_table [ (   {
>>   column_name [ WITH OPTIONS ] [ column_constraint [ ... ] ] |
>>   table_constraint } [, ... ] ) ] FOR VALUES 
>> partition_bound_spec *
>>   *partition_bound_spec* is:
>>

Re: [pgadmin-hackers] [pgAdmin4][Patch][RM_2400]: Columns with defaults set to NULL when removing contents after pasting in the edit grid

[Surinder Kumar]

Hi

Please find updated patch and review.

On Thu, May 18, 2017 at 7:36 PM, Joao Pedro De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hello Hackers,
>
> We reviewed the PR, and there are a lot of new lines of code in this
> patch, are we sure that we can have a good coverage of the functionality
> just by doing feature tests?
> Adding more code to a 3.5k+ lines file doesn't look like a good option, do
> you think it is possible to extract some of the functionality to their own
> files and have test around those functionalities?​
>
​To improve the code readability, reduce code complexity and to make code
testable, the code must be splitted component wise.
Here is my suggestion:

1. The code for classes like “SQLEditorView” and “SqlEditorController” can
be moved into two files like "editor_view.js and “editor_controller.js" and
called from within "sqleditor.js".

2. All utilities functions can be moved into separate utils file and can
write test cases.

3. Slickgrid listener functions such as:
​
onBeforeEditCell, onKeyDown,
​ ​
onCellChange, onAddNewRow
​
can be moved into
​ a file and write test cases around.

This needs discussion. Any suggestion?

>
> Do we really need to have an epicRandomString function in our code? Would
> it be better to use a library that would provide us that functionality out
> of the box?
>
We are using "epicRandomString function" to uniquely identify each record,
I talked to Harshal who is eliminating the use of this function and instead
maintaining counter for the rows added/updated/deleted.

> The functions this.applyValue in slick.pgadmin.editors.js that were
> change in this patch are exactly the same, can we extract that code into a
> single function instead of repeating the code?
>
​I have moved common logic into a new function.​

>
> Using feature tests is a good option to ensure that the integration of all
> the components of the application is working as expected, but in order to
> tests behaviors that are edge cases the Unit Tests are much cheaper to run
> and create.
>
​I will write test cases for functions once they are moved into their
separate files as discussed above.

>
> Thanks
> Joao & Shruti
>
>
> On Thu, May 18, 2017 at 1:22 AM, Surinder Kumar <
> surinder.ku...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> Please review the updated patch.
>>
>> On Wed, May 17, 2017 at 8:46 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Wed, May 17, 2017 at 3:08 PM, Surinder Kumar <
>>> surinder.ku...@enterprisedb.com> wrote:
>>>
 Hi Dave,

 *Implementation:*

 1) Took a flag 'is_row_copied' for each copied row:

  - to distinguish it from add/update row.
  - to write code specific to copied row only as it requires different
 logic than add row.

 2) After pasting an existing row:

  - If a user clear the cell value, it must set cell to [default] value
 if default value is explicitly given for column while creating table
 otherwise [null].

  - Again, if a user entered a value in same cell, then removes that
 value, set it to [null] (the same behaviour is for add row).

 3) Took a 2-dimensional array "grid.copied_rows" to keep track the
 changes of each row's cell so that changes made to each cell are
 independent and removed once data is saved.

 4) On pasting a row, the cell must be highlighted with light green
 colour, so triggers an addNewRow event. Now copied row will add to grid
 instead of re-rendering all rows again.

 5) Moved the common logic into functions.

 This patch pass the feature test cases and jasmine test case.
 Also I will add the test cases for copy row and send an updated patch.

 Please find attached patch and review.

>>>
>>> Looks good. I saw the following issues:
>>>
>>> - The "new" row is not available once I've created the first new row, or
>>> pasted some.
>>>
>> ​Fixed.
>>
>>>
>>> - Rows are pasted in reverse order.
>>>
>> ​Fixed.
>>
>>>
>>> - If I copy/paste a new row that has yet to be saved, none of the values
>>> are actually copied.
>>>
>> ​Fixed.​
>>
>>>
>>> - Attempting to save a row that contains all null/default values results
>>> in an invalid query:
>>>
>>> INSERT INTO public.defaults (
>>> ) VALUES (
>>> );
>>>
>>> I think the only answer here is to explicitly insert NULL into any
>>> columns *without* a default value.
>>>
>> ​I could not reproduce, However  #3 might have fixed it too.​
>>
>> Apart from this, I noticed epicRandomString(...) function doesn't
>> generate unique number always, due to this save and delete rows was
>> affected. Not all rows saved or deleted. The new function always returns a
>> unique random number.
>> Fixed.
>>
>>>
>>> Thanks.
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>>
>>
>> --
>> Sent via 

Re: [pgadmin-hackers] Update pgAdmin4 version on pgadmin.org from 1.4 to 1.5

[Dave Page]

Thanks, updated.

On Fri, May 19, 2017 at 1:21 PM, Harshal Dhumal <
harshal.dhu...@enterprisedb.com> wrote:

> Hi Dave,
>
> Text 1.4 is not updated to 1.5 on pgadmin.org
>
> [image: Inline image 2]
> --
> *Harshal Dhumal*
> *Sr. Software Engineer*
>
> EnterpriseDB India: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>



-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

[pgadmin-hackers] Update pgAdmin4 version on pgadmin.org from 1.4 to 1.5

[Harshal Dhumal]

Hi Dave,

Text 1.4 is not updated to 1.5 on pgadmin.org

[image: Inline image 2]
-- 
*Harshal Dhumal*
*Sr. Software Engineer*

EnterpriseDB India: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

[pgadmin-hackers] Build failed in Jenkins: pgadmin4-master-python35 #118

[pgAdmin 4 Jenkins]

See 


Changes:

[Dave Page] Fix RE-SQL for rules which got the table name wrong in the header 
and

--
[...truncated 254.56 KB...]
runTest 
(pgadmin.browser.server_groups.servers.databases.schemas.tests.test_schema_get.SchemaGetTestCase)
Check Schema Node URL ... ok
runTest 
(pgadmin.browser.server_groups.servers.databases.schemas.tests.test_schema_put.SchemaPutTestCase)
Check Schema Node URL ... ok
runTest 
(pgadmin.browser.server_groups.servers.databases.schemas.types.tests.test_types_add.TypesAddTestCase)
Add type under schema node ... ok
runTest 
(pgadmin.browser.server_groups.servers.databases.schemas.types.tests.test_types_delete.TypesDeleteTestCase)
Delete type under schema node ... ok
runTest 
(pgadmin.browser.server_groups.servers.databases.schemas.types.tests.test_types_get.TypesGetTestCase)
Get type under schema node ... ok
runTest 
(pgadmin.browser.server_groups.servers.databases.schemas.types.tests.test_types_put.TypesUpdateTestCase)
Update type under schema node ... ok
runTest 
(pgadmin.browser.server_groups.servers.databases.schemas.views.tests.test_views_add.ViewsAddTestCase)
Add view under schema node ... ok
runTest 
(pgadmin.browser.server_groups.servers.databases.schemas.views.tests.test_views_add.ViewsAddTestCase)
Add materialized view under schema node ... ok
runTest 
(pgadmin.browser.server_groups.servers.databases.schemas.views.tests.test_views_delete.ViewsDeleteTestCase)
Delete view under schema node ... ok
runTest 
(pgadmin.browser.server_groups.servers.databases.schemas.views.tests.test_views_delete.ViewsDeleteTestCase)
Delete materialized view under schema node ... ok
runTest 
(pgadmin.browser.server_groups.servers.databases.schemas.views.tests.test_views_get.ViewsGetTestCase)
Get view under schema node ... ok
runTest 
(pgadmin.browser.server_groups.servers.databases.schemas.views.tests.test_views_get.ViewsGetTestCase)
Get materialized view under schema node ... ok
runTest 
(pgadmin.browser.server_groups.servers.databases.schemas.views.tests.test_views_put.ViewsUpdateTestCase)
Update view under schema node ... ok
runTest 
(pgadmin.browser.server_groups.servers.databases.schemas.views.tests.test_views_put.ViewsUpdateTestCase)
Update materialized view under schema node ... ok
runTest 
(pgadmin.browser.server_groups.servers.databases.tests.test_db_add.DatabaseAddTestCase)
Check Databases Node URL ... ok
runTest 
(pgadmin.browser.server_groups.servers.databases.tests.test_db_delete.DatabaseDeleteTestCase)
Check Databases Node URL ... ok
runTest 
(pgadmin.browser.server_groups.servers.databases.tests.test_db_get.DatabasesGetTestCase)
Check Databases Node URL ... ok
runTest 
(pgadmin.browser.server_groups.servers.databases.tests.test_db_put.DatabasesUpdateTestCase)
Check Databases Node ... ok
runTest 
(pgadmin.browser.server_groups.servers.resource_groups.tests.test_resource_groups_add.ResourceGroupsAddTestCase)
Add resource groups ... ok
runTest 
(pgadmin.browser.server_groups.servers.resource_groups.tests.test_resource_groups_delete.ResourceGroupsDeleteTestCase)
Delete resource groups ... ok
runTest 
(pgadmin.browser.server_groups.servers.resource_groups.tests.test_resource_groups_put.ResourceGroupsPutTestCase)
Put resource groups ... ok
runTest 
(pgadmin.browser.server_groups.servers.resource_groups.tests.tests_resource_groups_get.ResourceGroupsGetTestCase)
Get resource groups ... ok
runTest 
(pgadmin.browser.server_groups.servers.roles.tests.test_role_add.LoginRoleAddTestCase)
Check Role Node ... ok
runTest 
(pgadmin.browser.server_groups.servers.roles.tests.test_role_delete.LoginRoleDeleteTestCase)
Check Role Node ... ok
runTest 
(pgadmin.browser.server_groups.servers.roles.tests.test_role_get.LoginRoleGetTestCase)
Check Role Node ... ok
runTest 
(pgadmin.browser.server_groups.servers.roles.tests.test_role_put.LoginRolePutTestCase)
Check Role Node ... ok
runTest 
(pgadmin.browser.server_groups.servers.tablespaces.tests.test_tbspc_add.TableSpaceAddTestCase)
Check Tablespace Node ... ok
runTest 
(pgadmin.browser.server_groups.servers.tablespaces.tests.test_tbspc_delete.TableSpaceDeleteTestCase)
Check Tablespace Node ... ok
runTest 
(pgadmin.browser.server_groups.servers.tablespaces.tests.test_tbspc_get.TablespaceGetTestCase)
Check Tablespace Node ... ok
runTest 
(pgadmin.browser.server_groups.servers.tablespaces.tests.test_tbspc_put.TableSpaceUpdateTestCase)
Check Tablespace Node ... ok
runTest 
(pgadmin.browser.server_groups.servers.tests.test_check_recovery.TestCheckRecovery)
Test for check recovery ... ok
runTest 
(pgadmin.browser.server_groups.servers.tests.test_dependencies_sql.TestDependenciesSql)
Test dependencies SQL file ... ok
runTest 
(pgadmin.browser.server_groups.servers.tests.test_dependents_sql.TestDependentsSql)
Test dependencies SQL file ... ok
runTest 

Re: [pgadmin-hackers] [Design update] Style guide for pgAdmin4

[Dave Page]

Hi

On Thu, May 18, 2017 at 6:57 PM, Shirley Wang  wrote:

> Hello!
>
> *Update on style guide location*
> We're going to move the agreed upon styles from Figma onto a webpage that
> will be accessible by everyone. We've decided to have a static site, with
> the html and css available to copy and paste. We'll add components to the
> style guide after we update the current UI with new styles.
>
> *Dropdown styles*
> I noticed that some drop downs include a 'x' next to the current option in
> the text field:
>
> The 'x' and the down arrow functionally do the same thing, and it seems
> redundant to have both. I would suggest removing the 'x' since selecting
> another option from the dropdown will automatically replace the current
> option.
>

No they don't. The x clears the current selection, and opens the combo. The
arrow will just open the combo. Both operations are valid (and in some
cases, required), but for clarity we probably should change it such that
the x doesn't open the combo.



>
> *Button toggles vs checkboxes*
> In some cases within dialogs, a toggle is used to define yes or no, while
> in other cases a radio button is used to define yes or no:
>
> It does seem like 'Connect now?'  and 'Save password' (in 'Connection' tab
> within 'Create - Server') are the only places where checkboxes are used, is
> there a reason why these are different? I think we should just choose one,
> unless there was a specific reason as to why both check boxes and toggles
> are used.
>

I think it was just because that was one of the very first dialogues
written, when we were in POC mode. It should be fixed.

Thanks!

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

[pgadmin-hackers] pgAdmin 4 commit: Fix RE-SQL for rules which got the table name wrong i

[Dave Page]

Fix RE-SQL for rules which got the table name wrong in the header and DROP 
statement. Fixes #2422

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=04ce72a6aeeacc2bca7f3a7cfa2ab3a97419b2fe

Modified Files
--
.../servers/databases/schemas/tables/templates/rules/sql/create.sql   | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)


-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

[pgadmin-hackers] pgAdmin 4 v1.5 Released!

[Dave Page]

The pgAdmin Development Team are pleased to announce the release of
pgAdmin 4 version 1.5. This release of pgAdmin 4 includes over 20 bug
fixes and improvements:

Bug #2225: Object creation options are visible even if the object type is hidden
Bug #2253: Download as CSV often doesn't do anything
Bug #2257: Query tool - Insert row doesn't use default values
Bug #2271: Disable Trigger issue
Bug #2284: Cannot create table with integer name
Bug #2292: pgAdmin4 sometimes opens sessions on all databases on the box
Bug #2314: Multiple issues with csv download
Bug #2315: Sorting by size is broken
Bug #2318: Foreign Table create script puts columns in alphabetical
order instead of the actual order of the columns as the table was
originally created
Bug #2331: Browser tree - Adding nodes at wrong location
Bug #2336: Browser tree - Elements inode not get updated on refresh
Bug #2339: Horizontal scrolling in the treeview
Bug #2350: Default parameters in function PgAdmin 1.4
Bug #2354: select other language,backup not run
Bug #2356: Delete row server error not shown to GUI user
Bug #2360: Unicode identifiers in CSV-export of JSON fields
Bug #2369: Query tool: add support to files with BOM
Bug #2376: Internal server error displayed If click on properties
section of newly created sequence on PG-10
Bug #2377: [Web based] An Error occurred whilst rendering the table
displayed on server activity table for PG-10 on windows
Bug #2378: [Desktop Runtime EDB package] Internal server error
displayed if Click on Statistics tab for PG-10 server on windows
Bug #2399: [View Data] - Disabled row's background color disappeared on save
Bug #2405: XSS in Process Watcher via Backup Job Filename
Bug #2410: [Desktop community installer] pg-10 compatibility issues
Feature #2216: Select a column in query output

For more information, checkout the online documentation
[https://www.pgadmin.org/docs/pgadmin4/1.x/], the screenshots
[https://www.pgadmin.org/screenshots/], and of course the download
page [https://www.pgadmin.org/download/].



-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

[pgadmin-hackers] pgAdmin 4 commit: Tag REL-1_5 has been created.

[Dave Page]

Tag REL-1_5 has been created.
View: https://git.postgresql.org/gitweb?p=pgadmin4.git;a=tag;h=refs/tags/REL-1_5

Log Message
---
Tag 1.5
-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] Fix for RM2421 [pgAdmin4][patch]

[Harshal Dhumal]

Hi,

On Thu, May 18, 2017 at 7:57 PM, Joao Pedro De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hello Harshal,
>
> We review the patch and have some questions:
> 1) Is there any particular reason to initialize variables and functions in
> the same place? We believe that it would be more readable there were no
> chaining of variable creation, specially if those variables are functions.
> Check line:
>
That function is only going to be used in checkNumeric function (in case of
Number control) and checkInt function (in case of Integer control) so
declared them locally.
Anyway I'm going to refactor both the controls as Number and Integer shares
some common properties.

+++ b/web/pgadmin/static/js/backform.pgadmin.js
> @@ -1528,7 +1528,18 @@
>max_value = field.max,
>isValid = true,
>intPattern = new RegExp("^-?[0-9]*$"),
> -  isMatched = intPattern.test(value);
> +  isMatched = intPattern.test(value),
> +  trigger_invalid_event = function(msg) {
>
> ​
> 2) The functions added in both places look very similar, can they be
> merged and extracted? We are talking about the trigger_invalid_event
> function.
>
Yes they can be merged. As of now both NumericControl and IntegerControl
are derived from InputControl. Ideally
only NumericControl should be derived from InputControl and IntegerControl
should be derive from NumericControl.



> 3) The following change is very similar to the trigger_invalid_event, was
> there a reason not to use it?
>
Below code triggers "model valid" event; opposite to "model invalid" event (
trigger_invalid_event)

> +++ b/web/pgadmin/static/js/backform.pgadmin.js
> @@ -1573,25 +1584,23 @@
>  this.model.errorModel.unset(name);
>  this.model.set(name, value);
>  this.listenTo(this.model, "change:" + name, this.render);
> -if (this.model.collection || this.model.handler) {
> -  (this.model.collection || this.model.handler).trigger(
> - 'pgadmin-session:model:valid', this.model, 
> (this.model.collection || this.model.handler)
> -);
> +// Check if other fields of same model are valid before
> +// triggering 'session:valid' event
> +if(_.size(this.model.errorModel.attributes) == 0) {
> +  if (this.model.collection || this.model.handler) {
> +(this.model.collection || this.model.handler).trigger(
> +   'pgadmin-session:model:valid', this.model, 
> (this.model.collection || this.model.handler)
> +  );
> +  } else {
> +(this.model).trigger(
> +   'pgadmin-session:valid', this.model.sessChanged(), this.model
> +  );
> +  }
>
> ​
> 4) We also noticed that the following change sets look very similiar. Is
> there any reason to have this code duplicated? If not this could be a good
> time to refactor it.
>
As said earlier in response of point 2 code duplication is because the way
controls are derived.


> +++ b/web/pgadmin/static/js/backform.pgadmin.js
> @@ -1528,7 +1528,18 @@
>
> @@ -1573,25 +1584,23 @@
>
> @@ -1631,7 +1640,18 @@
>
> @@ -1676,25 +1696,23 @@
>
> ​
>
> Thanks
> Joao & Shruti
>
> On Thu, May 18, 2017 at 6:01 AM, Harshal Dhumal <
> harshal.dhu...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> Please find attached patch for RM2421
>>
>> Issue fixed: 1. Integer/numeric Validation is not working properly.
>> 2. Wrong CPU rate unit
>> --
>> *Harshal Dhumal*
>> *Sr. Software Engineer*
>>
>> EnterpriseDB India: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>
>> --
>> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>
>>
>