| Age | Commit message (Collapse) | Author | 
 | 
 | 
 | 
af83/5281-workbench-import-structural-bug-reading-zip
5281 workbench import structural bug reading zip
 | 
 | 
Update TableBuilderHelper to allow the `selectable` param to be a
lambda, thus allowing us to have row-based granularity.
 | 
 | 
Add specs for the view.
Refactor to come
 | 
 | 
- CR change from #5006, severity of zip file messages :warning -> :error
  - View specs for showing import_messages
 | 
 | 
 | 
 | 
 | 
 | 
 | 
 | 
remove the Chouette namespace
off of the construction of link in the app and in other places.
Added a small change in the reflex sync to set the stop_area_referential in the initialization of the object to be in sync with the work on the objectids (need a objectid_formatter)
 | 
 | 
4755 page for validate referential
 | 
 | 
 | 
 | 
Step 2: Added validation of reference->workbench->organisation consistency
            Made all specs pass
            Chased bug #4826
 | 
 | 
 | 
 | 
The `TableBuilderHelper` now requires you to specify a `link_to`
attribute in a column you want to make a link out of instead of doing it
automagically as before.
Update the tests to do this and continue to output the right `<a>`s for
links.
 | 
 | 
This reverts commit 28db706443a912e8355e4c48488dc40c403e7f76.
Turns out we didn't need to be able to pass an arbitrary number of
arguments to the lambda after all. The URL helper objects necessary in
addition to the first argument to the lambda can be retrieved from the
view context directly instead of passing them into the block as
parameters (which would actually make things more difficult, because the
block is called in the `TableBuilderHelper`, thus outside the scope of
the view).
 | 
 | 
This enables us to pass multiple objects into the lambda, so that we can
for example build a URL using a Rails helper using multiple objects.
Example:
    column.link_to(referential, item)
    lambda do |referential, item|
      some_path(referential, item)
    end
 | 
 | 
This tells the markup assembler whether or not this column should be
wrapped in a link (`<a>`). It intends to serve the same purpose as
`TableBuilderHelper#column_is_linkable`, but at the column level. The
idea is to remove that method when these links only operate by column
and not by pre-defined values like before/now.
 | 
 | 
This parameter will be used as the `href` to link the column value
somewhere.
We give it a lambda because this makes it easier to call any method on
the row object.
This means the accessor needs to take the object as an argument, like in
the `#value` method, because we don't have a better way to handle that
(it can't be done at initialisation time because at that point we don't
have row objects, we have a collection).
 | 
 | 
Rename this to `referential` to be more generic. This is because we
could be passing both `Referential`s and `LineReferential`s into this
parameter.
In `CompaniesController`, we use a `LineReferential` while in
`ReferentialCompaniesController` we use `Referential`.
Refs #3479
 | 
 | 
Instead of getting the referential to use when building the polymorphic
URL from the `UserContext`, pass in a referential directly.
The old code that used `user_context.context[:referential]` relied on
the fact that `ApplicationController#pundit_user` was defined as
follows:
     def pundit_user
       UserContext.new(current_user, referential: self.try(:current_referential))
     end
(We pass `pundit_user` into `CustomLinks` from
`TableBuilderHelper#build_links`.)
However, Robert's change 747d333ffbcc8ee0c9f1daf93ccca32799434e04
removes the `current_referential` call from `#pundit_user`. In
`CustomLinks`, we actually always want to be using
`current_referential`.
For example, on `Companies#index` (/line_referentials/:id/companies),
`CustomLinks` fails to build a correct #show link because
`user_context.context[:referential]` is `nil`, when it should instead be
a `LineReferential` object, that gets returned by the
`#current_referential` helper method. Sure, `#current_referential` is
hard to understand, so maybe we'll change that around in the future, but
this at least allows us to use the current referential in `CustomLinks`.
Refs #3479
 | 
 | 
The name of this method changed, but wasn't updated in the `describe`
label.
 | 
 | 
 | 
 | 
These two specs were failing because of the line:
    current_referential ||= nil
which would set `current_referential` to `nil` even after updating the
object being stubbed to `helper`.
Change the specs to stub the method on the correct object now that
`TableBuilderHelper::URL` doesn't depend on `current_referential`.
Update the faulty line in question to not clobber `current_referential`,
but still give us the `nil` value we were looking for when the helper
method isn't defined.
Refs #3479
 | 
 | 
Policy Refactoring and Policy Test Completion
   - All policies (and all permissions) under test.
   - Common patterns and potential problems identified...
   - ... and documented in DEVNOTES.md
   - some simply refactorings
 | 
 | 
-  All permissions tied to `!archived?`
   -  Tests adapted
   -  Policies refactored
   ?  Is `create?` permission bound to `organisation_match?`
 | 
 | 
models and actions
 - ApplicationPolicy nondestructive permission depend on model existance
 - ApplicationPolicy destructive permission default to `false`
 - Tied Policy permissions at ApplicationPolicy Level: edit? → update?, new? → create?, index? → show?
 - ApplicationPolicy convenience methods `delete?` & `authorizes_action?(action)`
 - Refactoring of `spec/helpers/table_builder_helper_spec.rb` accordingly
 - Stubbing scope in specs (cannot switch to referential with a `build_stubbed` instance)
 | 
 | 
 | 
 | 
to moving authoriation BL into policies
 | 
 | 
 | 
 | 
 | 
 | 
 | 
 | 
We're how have a separate module for the box containing links to perform
actions on selections of multiple objects in the table. I had put this
in `MultipleSelectionToolboxHelper`.
Refs #3479
 | 
 | 
Remove the hard-coded IDs and replace them with the test referential's
ID. I guess I had copied the output from the spec output, because the
test then passed when I ran it individually, causing me to not catch the
error. Only saw it when I ran the full test suite.
Refs #3479
 | 
 | 
These are all finished.
Refs #3479
 | 
 | 
Instead of building our context with an `OpenStruct`, use the
`UserContext` class that actually exists and represents the thing that
we want to represent.
I had copied the `OpenStruct` way of doing from
"spec/support/pundit/policies.rb" this before I found out that
`UserContext` existed.
Refs #3479
 | 
 | 
The delete button confirmation text has changed now that we're getting
the delete link from `ReferentialDecorator`. Update the spec to reflect
the new state of the code.
Refs #3479
 | 
 | 
We need the ability to say that an entire table is not sortable. By
default, all tables are sortable, and individual columns can have
sorting deactivated. This blanket deactivates sorting for all columns.
Take our `sortable` argument and pass it to `#thead`. Then
`#build_column_header` takes that value into account when creating a
header column value.
The actual label used in the column header is determined by the old
`#column_header_label` method. We've now moved it into the
`TableBuilderHelper::Column` class because it makes more sense there.
The method will now return the column's `:name` property if it was
defined, and otherwise gets the translation for `:key` as before.
Add a test on `TableBuilderHelper` that verifies that a table with
`sortable: false` returns the correct HTML without sorting links.
Refs #3479
 | 
 | 
In order to enable access to `CompanyDecorator#action_links`, wrap the
companies collection passed to the table builder in the test in the
decorator.
Specify the class that `CompanyDecorator` decorates explicitly. Since
`Chouette::Company` is namespaced, Draper can't infer the model from the
decorator class name.
Refs #3479
 | 
 | 
We're now using the `current_referential` method in
`TableBuilderHelper::URL` instead of in `TableBuilderHelper`. Thus it
couldn't get `current_referential` and caused an error. Move the mock to
`TableBuilderHelper::URL` to give it access to the method.
Refs #3479
 | 
 | 
A couple more links should appear in the gear menu thanks to the
inclusion of the `ReferentialDecorator#action_links`'s links in the
menu.
The test still fails, though, because the destroy link that comes out of
the decorator isn't formatted correctly (it looks like all the other
links instead of having a special icon).
Refs #3479
 | 
 | 
The `ReferentialDecorator` duplicates the [:archive, :unarchive,
:delete] actions, so we should remove these from our test like we did
previously in the "workbenches/show.html.slim" template.
Refs #3479
 | 
 | 
Instead of mapping over the objects and decorating them manually, use
the `ModelDecorator` collection decorator to do it. This is what the
actual code does, so we should be doing the same in our test.
Refs #3479
 | 
 | 
In order to properly build the table, the objects in the collection
passed to the table builder need to be decorated to add the
`#action_links` method.
Need to fix the other test too, but we don't have a `CompanyDecorator`
yet.
Refs #3479
 | 
 | 
Refs #3479
 | 
 | 
In `#actions_after_policy_check`, we weren't correctly including actions
that weren't handled by the prior policy checks. Thus actions that
weren't [:delete, :edit, :archive, :unarchive] wouldn't get included in
the resulting list of actions.
Fix my logic error.
Refs #3479
 | 
 | 
These params are not relevant to this test, they just clutter things up.
I had originally added them because the `/workbenches/:id` page used
them and I wanted to mimic the table builder result as closely as
possible.
Refs #3479
 | 
 | 
I had added these last week uncommitted, but still haven't gotten to
them. Figure I should just commit them to save them and remove them when
I get to them.
Refs #3479
 | 
 | 
Add a few extra newlines to make these sections a bit easier to read.
Refs #3479
 | 
 | 
Instead of making the columns a hash where the types get all confused
and mussed and we have to type check wherever we use the column value,
make the columns into their own real type.
Adds a new `Column` class to the helper module which lets us interact
with table column data in a much more natural way. Here, when mapping
over the column list, instead of getting a key and value which could be
whatever, we get a real `Column` object that we can call methods on to
get the appropriate data.
It's still not perfect, as evidenced by the `#tbody` method, which has
remained largely the same, but at least we're not checking
`attribute.is_a?(Hash)` like I had tried before.
Update the tests to use the new `TableBuilderHelper::Column` syntax for
defining columns.
Refs #3479
 | 
 | 
Provides an option to set a column as non-sortable when defining the
column in the `table_builder` call.
Not written well, but it gets the idea across.
We allow column hash values to be hashes themselves (while continuing to
allow them to be `Proc`s and strings, this mixing of types is what I
really don't like). In that column-level hash, we can pass
`sortable: false` to disable sorting on that column. By default, sorting
is turned on for all columns.
Change the method signature of `sortable_columns`. Rename it to
`build_column_header`, as it will now generate a correct column header
regardless of whether the column is supposed to be sortable or not. I
moved some dependencies outside of the method, like `collection` (since
we only need the model from the collection), and the sort fields on
`params`. Unfortunately wasn't able to completely extricate `params` for
this method. Left the `params.merge` call that builds a link in this
method. Ideally I'd like that removed so that the method no longer
depends on global variables.
In `build_column_header`, if the column is not sortable, we return the
label defined by the key in the columns hash defined by the
`table_builder` caller. Otherwise, we build some HTML that includes
links on a couple of arrow buttons to enable sorting.
In `#thead`, we check to see if the columns hash value has a `sortable`
defined, in which case that flag gets used to determine what the column
header will be. I renamed the block variables here to be more
descriptive. Still not a huge fan of `attribute` as a name, though,
because it can now also be a Hash. Another aspect of this change that I
don't like.
In `#tbody`, needed to add a few extra lines to check whether
`attribute` is a Hash and get the real attribute if so. Really don't
like that either because it doesn't look good.
Update the sortable spec to pass a hash as the value of the column that
should not be sorted. It defines a `:sortable` key that tells the
builder not to allow this column to be sorted.
Refs #3479
 |